Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change replaces Irys uploads with bundler requests, embeds chained ref certificates and pusher signature metadata, adds anchor lifecycle persistence, and exposes gateway-based Arweave transaction verification. ChangesArweave integrity flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant git_receive_pack
participant issue_ref_certificate
participant Bundler
participant verify_anchor_endpoint
participant verify_anchor
participant ArweaveGateway
participant Db
Client->>git_receive_pack: Authenticated push request
git_receive_pack->>issue_ref_certificate: Pass pusher signature and proof
issue_ref_certificate->>Db: Store chained certificate
git_receive_pack->>Bundler: POST /v1/tx with certificate anchor
Bundler-->>git_receive_pack: Return transaction ID
Client->>verify_anchor_endpoint: GET /api/v1/arweave/verify/{tx_id}
verify_anchor_endpoint->>verify_anchor: Verify transaction ID
verify_anchor->>ArweaveGateway: GET gateway/{tx_id}
ArweaveGateway-->>verify_anchor: Return anchored payload
verify_anchor->>Db: Load predecessor certificate
verify_anchor-->>Client: Return validity, errors, and certificate
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
0801800 to
bd09c35
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gitlawb-node/src/arweave.rs`:
- Around line 351-373: Update the prev-linkage validation in verify_anchor to
fetch and hash the local certificate at sequence c.seq - 1, rather than the
newest certificate returned by get_most_recent_cert. Only perform the comparison
when that predecessor exists, while preserving the existing mismatch error
handling and payload hashing behavior.
- Around line 319-343: Update the signature decoding in the certificate
verification flow to use the URL-safe, no-padding base64 engine matching
node_keypair.sign_b64 output, while preserving the existing 64-byte validation
and error-result handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fee43d49-2e4b-4bbd-9921-8248f57fea48
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Use the configured gateway's data URL when verifying an anchor
crates/gitlawb-node/src/arweave.rs:265
arweave_gatewaydefaults tohttps://arweave.net, but this requests/v1/tx/{id}, which is the bundler API used for uploads rather than an Arweave gateway data URL. Consequently every normally uploaded anchor is reported invalid with the documented default configuration. Fetch the item through the gateway's data path (or add a separately named bundler-read configuration), and cover the default configuration rather than a mock of the bundler path. -
[P1] Bind each anchor to the certificate for its own ref update
crates/gitlawb-node/src/api/repos.rs:1306
Certificates are issued once per update above this block, but every iteration subsequently reads the repository-wide latest certificate. In a multi-ref push, all permanent anchors therefore embed the last update's certificate; another completed push can also win the asynchronous race. Sinceverify_anchornever compares the certificate's repo/ref/old/new fields with the outer anchor, it returns valid for an anchor whose advertised transition was never signed. Preserve the returned certificate per update and reject a mismatch during verification. -
[P1] Preserve certificate history and fail closed on a missing predecessor
crates/gitlawb-node/src/db/mod.rs:2013
The existing(repo_id, ref_name)upsert overwrites the predecessor whenever that ref is pushed again, while the newseq/prevdesign requires that predecessor to remain available.verify_anchorthen silently skips the check whenget_cert_by_seqreturnsNoneor errors, so an ordinary repeated push produces a truncated chain reported as valid. Make chain entries append-only and treat an unavailable declared predecessor as invalid (or explicitly unverifiable). -
[P2] Allocate chain sequence numbers atomically
crates/gitlawb-node/src/cert.rs:31
Sequence allocation is a read-then-increment with no transaction, lock, or unique(repo_id, seq)constraint. Concurrent successful pushes can receive the same sequence and predecessor;get_cert_by_seqthen selects an arbitrary row. This makes a signed chain nondeterministic under normal concurrent traffic. Allocate the sequence transactionally and enforce uniqueness, with retry on collision. -
[P1] Do not describe raw signature bytes as a verifiable pusher authorization proof
crates/gitlawb-node/src/auth/mod.rs:252
Only the 64-byte Ed25519 signature is persisted. The RFC 9421Signature-Input, covered component values, method/path, and content digest are discarded, andverify_anchornever verifiespusher_sig. A third party therefore cannot reconstruct the signing string or bind these bytes to this push, yet the endpoint can report the anchor valid. Persist a complete verifiable authorization artifact and validate it, or remove the proof/verification claim. -
[P1] Bound the untrusted response read on the public verification route
crates/gitlawb-node/src/arweave.rs:279
The new unauthenticated, unthrottled route buffers the full gateway response withresp.bytes()before attempting JSON parsing. A caller can repeatedly select large data items and force corresponding memory and bandwidth consumption on the node. Apply a strict response-size limit (and a route-appropriate rate limit) before buffering or parsing the body. -
[P3] Implement the promised anchor failure lifecycle instead of dropping failed uploads
crates/gitlawb-node/src/api/repos.rs:1324
Upload failures only log a warning; no anchor row is created, retried, confirmed, or marked failed. The new pending/confirmed/failed methods are unused, and success-only rows are always inserted aspending. This does not meet the linked issue's stated retry/visible-gap acceptance criterion, so transient bundler failures silently leave history unanchored. Persist pending work before upload and drive it through bounded retry and terminal status handling. -
[P2] Keep the documented anchoring configuration working during the rename
crates/gitlawb-node/src/config.rs:91
This removesGITLAWB_IRYS_URLwithout a fallback, while both.env.exampleandREADME.mdstill instruct operators to set it. Upgrading an existing documented deployment leavesbundler_urlempty and silently disables both anchoring paths. Support the legacy variable for a deprecation period or make the migration explicit and update all operator documentation in the same change. -
[P3] Expose the new signed fields through the certificate API
crates/gitlawb-node/src/api/certs.rs:45
The certificate signing payload now includesseq,prev, andpusher_sig, but both list and get responses omit all three fields. Consumers of the established certificate API (includinggl cert) therefore cannot reconstruct the signed payload or inspect chain continuity after this change. Serialize the new fields and update the client display/verification path accordingly.
|
@Gravirei please rebase to main and fix conflicts |
c94e8ed to
ae4f5fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/gitlawb-node/src/test_support.rs (1)
4983-4988: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake
seed_certproduce chain-valid fixtures.This helper creates the 10- and 55-certificate datasets, but every certificate has
seq: 1and a zero predecessor. The tests therefore cannot catch regressions that ignore monotonic ordering orprevlinks. Pass sequence/predecessor values through the helper or add a dedicated chained fixture.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gitlawb-node/src/test_support.rs` around lines 4983 - 4988, Update the seed_cert fixture helper so generated certificates form a valid chain: assign increasing sequence values and set each certificate’s prev field to the preceding certificate’s identifier or digest, with the first certificate using the chain’s root predecessor. Ensure both the 10- and 55-certificate datasets exercise monotonic ordering and linked predecessors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gitlawb-node/src/arweave.rs`:
- Around line 292-293: Update the payload parsing in verify_anchor_endpoint so
serde_json::from_slice failure is handled as an invalid verification result
rather than propagated as an internal error. Return VerifyResult with valid set
to false and an appropriate error string, while preserving the existing JSON
parsing path.
- Around line 281-293: Update the response handling around resp.bytes() in the
verification flow to enforce the 1 MiB limit before unbounded buffering: reject
any Content-Length above 1_048_576, and stream or otherwise cap reads so
responses without a trustworthy length header cannot exceed the limit. Preserve
the existing invalid VerifyResult fields and JSON parsing for accepted payloads.
In `@crates/gitlawb-node/src/db/mod.rs`:
- Around line 923-938: Migration version 13 must backfill distinct
per-repository sequence values before enforcing uniqueness. In the migration’s
`stmts` array, add an update that assigns deterministic, non-colliding `seq`
values to existing `ref_certificates` rows grouped by `repo_id`, then create
`idx_ref_certs_repo_seq`; preserve the existing column additions and append-only
index changes.
- Around line 5157-5162: Ensure each certificate created through make_cert
receives a unique seq value before insertion, either by incrementing it within
make_cert or overriding it at every test call site. Update the affected
certificate setup so list_ref_certificates_respects_limit and
insert_ref_certificate_append_only use distinct sequence numbers and avoid the
(repo_id, seq) uniqueness conflict.
---
Nitpick comments:
In `@crates/gitlawb-node/src/test_support.rs`:
- Around line 4983-4988: Update the seed_cert fixture helper so generated
certificates form a valid chain: assign increasing sequence values and set each
certificate’s prev field to the preceding certificate’s identifier or digest,
with the first certificate using the chain’s root predecessor. Ensure both the
10- and 55-certificate datasets exercise monotonic ordering and linked
predecessors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e15d3f3-7966-4b86-a8fa-640f7c92e10a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/certs.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/main.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/gitlawb-node/src/api/events.rs
- crates/gitlawb-node/src/server.rs
- crates/gitlawb-node/src/api/arweave.rs
- crates/gitlawb-node/src/config.rs
- crates/gitlawb-node/src/cert.rs
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Backfill legacy certificate sequence numbers before adding the unique index
crates/gitlawb-node/src/db/mod.rs:910
Migration 12 gives every existing certificateseq = 1, but migration 13 then creates a unique(repo_id, seq)index. Version 10 only deduplicated(repo_id, ref_name), so any existing repository with certificates for two refs has duplicate(repo_id, 1)rows and cannot complete this migration or start. This is also immediately exposed by the changedseed_certfixture, which inserts tenseq = 1rows and makeslist_certs_respects_limit_paramfail. Assign deterministic per-repository sequence/chain values before creating the index and update the fixture. -
[P1] Enforce the verification response limit before buffering the gateway body
crates/gitlawb-node/src/arweave.rs:282
The new unauthenticated verification route callsresp.bytes()and only checks the 1 MiB limit after the whole Arweave object has been downloaded and allocated. A caller can select a huge or chunked gateway item and exhaust node memory/bandwidth before the rejection occurs. Stream into a capped reader (and reject a known excessive content length up front) instead of buffering first. -
[P1] Restore cryptographic verification to
gl cert show
crates/gl/src/cert.rs:151
This branch has drifted from its base and removes the base's--verify,--expect-node, Ed25519 verification routine, and tests, although the command is still documented as verifying a certificate. It now only prints a proposed payload and returns success for a modified or self-signed certificate. Please rebase without reverting that fail-closed CLI contract, then update its canonical payload forseq,prev, andpusher_sig. -
[P2] Hold the certificate-chain lock through allocation and insertion
crates/gitlawb-node/src/db/mod.rs:2160
pg_advisory_xact_lockis transaction-scoped, but this standalone pooled query commits beforeissue_ref_certificatereads the previous certificate or inserts the new one. Concurrent pushes can therefore select the same sequence; with three or more contenders the single retry can collide again, leaving an accepted push without its certificate/anchor evidence. Use one acquired connection/transaction for the lock, predecessor read, and insert (or a robust serialization/retry strategy). -
[P2] Bind the embedded certificate to the enclosing Arweave anchor
crates/gitlawb-node/src/arweave.rs:303
Verification checks the copied certificate signature but never compares the untrusted outerrepo,owner_did, ref, SHAs, or node DID to that certificate. An attacker can publish a payload with a valid public certificate while claiming a different ref update and receivevalid: true. Reject field mismatches (and match a locally recorded transaction too if this endpoint is meant to validate local anchors). -
[P2] Keep
gl statuscompatible with the remote created bygl init
crates/gl/src/status.rs:146
This branch regresses the base's multi-remote lookup: the status command now accepts only agitlawb://fetch URL onorigin, whilegl initadds the same URL under thegitlawbremote. Immediately after the supported init flow,gl statusreports that the repository is not a Gitlawb repo and skips the PR/issue queries. Rebase without reverting the base's lookup for thegitlawbremote and other Gitlawb fetch/push URLs. -
[P2] Do not hard-code
mainafter a plaingit init
crates/gl/src/init.rs:41
This branch reverts the base's branch/commit-state handling. Plaingit inithonors the user'sinit.defaultBranch, but the command unconditionally instructsgit push gitlawb main. Onmaster, feature, detached, or unborn HEADs that instruction either targets a nonexistent/wrong ref or fails before the first commit. Rebase without dropping the previous branch/commit-state handling, or initializemainwith the compatibility fallback. -
[P2] Preserve the legacy command-line spelling during the bundler rename
crates/gitlawb-node/src/config.rs:75
The environment fallback runs only after Clap parses arguments. Existing operators invokinggitlawb-node --irys-url …now receive an unknown-argument startup error even though the PR claims compatibility forGITLAWB_IRYS_URL. Add a deprecated long alias or normalize a retained legacy option as well as the environment variable. -
[P2] Do not remove unrelated CI safety gates from this anchoring change
.github/workflows/pr-checks.yml:189
This is stale-base drift rather than part of the Arweave feature: the branch deletes the shipped Windows CLI test lane and the onlygitlawb-coredependency-purity gate, together with its allowlist and checker script. That removes platform regression visibility and a supply-chain control for the shared cryptographic core; rebase without deleting these protections, or justify and replace them in a separately scoped change.
ae4f5fc to
e9a1a5e
Compare
|
@coderabbitai please make an intensive review of this changes |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/gitlawb-node/src/api/events.rs (1)
429-435: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftUse valid per-repository certificate-chain fixtures.
crates/gitlawb-node/src/api/events.rs#L429-L435: replace the process-global sequence generator with per-chain fixture state.crates/gitlawb-node/src/api/events.rs#L447-L452: deriveprevfrom the preceding certificate instead of always using the genesis hash.crates/gitlawb-node/src/test_support.rs#L1481-L1486: avoid assigning a global sequence to an otherwise standalone certificate.crates/gitlawb-node/src/test_support.rs#L4967-L4973: make sequence generation scoped to a repository/chain.crates/gitlawb-node/src/test_support.rs#L4990-L4995: generate matching predecessor hashes for multi-certificate fixtures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gitlawb-node/src/api/events.rs` around lines 429 - 435, Replace the process-global NEXT_FCERT_SEQ/ref_cert_seq state in crates/gitlawb-node/src/api/events.rs:429-435 with sequence state scoped to each certificate chain; update the certificate construction at crates/gitlawb-node/src/api/events.rs:447-452 to derive prev from the preceding certificate. In crates/gitlawb-node/src/test_support.rs:1481-1486, leave standalone certificates without a global sequence; in lines 4967-4973, scope sequence generation to the repository or chain; and in lines 4990-4995, generate predecessor hashes that match the preceding certificates in multi-certificate fixtures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/gitlawb-node/src/api/certs.rs`:
- Around line 55-57: The certificate JSON responses in
crates/gitlawb-node/src/api/certs.rs must include complete pusher-signature
metadata. Update both the listed-certificates response at lines 55-57 and the
single-certificate response at lines 98-100 to include signature_input,
content_digest, and request_path alongside the existing pusher_sig fields.
---
Nitpick comments:
In `@crates/gitlawb-node/src/api/events.rs`:
- Around line 429-435: Replace the process-global NEXT_FCERT_SEQ/ref_cert_seq
state in crates/gitlawb-node/src/api/events.rs:429-435 with sequence state
scoped to each certificate chain; update the certificate construction at
crates/gitlawb-node/src/api/events.rs:447-452 to derive prev from the preceding
certificate. In crates/gitlawb-node/src/test_support.rs:1481-1486, leave
standalone certificates without a global sequence; in lines 4967-4973, scope
sequence generation to the repository or chain; and in lines 4990-4995, generate
predecessor hashes that match the preceding certificates in multi-certificate
fixtures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5645b021-f503-4733-92c1-424a3ae2349e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/certs.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/main.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
|
@coderabbitai make an intensive review on this please |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/gitlawb-node/src/arweave.rs (1)
303-304: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNon-JSON gateway payload still returns 500 instead of
valid: false.serde_json::from_slice(&body_bytes)?propagates throughverify_anchor_endpoint(mapped toAppError::Internal), inconsistent with the other graceful branches and the "could be JSON or raw bytes" comment. Convert a parse failure intoVerifyResult { valid: false, .. }with an error string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gitlawb-node/src/arweave.rs` around lines 303 - 304, Update the anchor parsing in verify_anchor_endpoint so serde_json::from_slice failures are handled as an invalid verification result rather than propagated as AppError::Internal. Return VerifyResult with valid set to false and an error string for non-JSON payloads, while preserving the existing successful JSON path and other graceful branches.
🧹 Nitpick comments (2)
crates/gitlawb-node/src/server.rs (1)
222-228: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider throttling the unauthenticated verify route. Each
GET /api/v1/arweave/verify/{tx_id}triggers an outbound gateway fetch plus a DB lookup with no auth or per-IP brake, so it's an amplification/DoS surface (node → gateway) reachable by anonymous callers. Given the other cost-bearing routes here carry a per-IPIpRateLimiter, consider wrappingarweave_routessimilarly. (tx_idis a fixed-host path segment, so this is a load concern, not SSRF.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gitlawb-node/src/server.rs` around lines 222 - 228, Wrap the arweave_routes router, including GET /api/v1/arweave/verify/{tx_id}, with the existing per-IP IpRateLimiter used by other cost-bearing routes. Preserve the current list_anchors and verify_anchor_endpoint handlers while ensuring anonymous requests are throttled before triggering gateway or database work.crates/gitlawb-node/src/db/mod.rs (1)
2834-2870: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
cert_idis never persisted on anchor rows.RecordAnchorInputV2has nocert_idfield andrecord_arweave_anchor's INSERT omits it, so thecert_idcolumn added in migration v12 stays NULL for every anchor even thoughlist_arweave_anchors/list_pending_anchorsproject it. The push path inapi/repos.rsalready has the issued certificate in scope (ref_certs_clone), so the anchor→certificate DB linkage this column was added for is currently unreachable. Consider threading the cert id through so audits can join anchors to their certs. (gateway_urlon the input is likewise accepted but ignored by this function.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gitlawb-node/src/db/mod.rs` around lines 2834 - 2870, The anchor record flow does not persist the issued certificate ID. Add a cert_id field to RecordAnchorInputV2, pass the corresponding ID from the push path using ref_certs_clone, and include it in record_arweave_anchor’s INSERT and bindings so cert_id is stored on each anchor row; also remove or persist gateway_url consistently instead of silently ignoring it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@crates/gitlawb-node/src/arweave.rs`:
- Around line 303-304: Update the anchor parsing in verify_anchor_endpoint so
serde_json::from_slice failures are handled as an invalid verification result
rather than propagated as AppError::Internal. Return VerifyResult with valid set
to false and an error string for non-JSON payloads, while preserving the
existing successful JSON path and other graceful branches.
---
Nitpick comments:
In `@crates/gitlawb-node/src/db/mod.rs`:
- Around line 2834-2870: The anchor record flow does not persist the issued
certificate ID. Add a cert_id field to RecordAnchorInputV2, pass the
corresponding ID from the push path using ref_certs_clone, and include it in
record_arweave_anchor’s INSERT and bindings so cert_id is stored on each anchor
row; also remove or persist gateway_url consistently instead of silently
ignoring it.
In `@crates/gitlawb-node/src/server.rs`:
- Around line 222-228: Wrap the arweave_routes router, including GET
/api/v1/arweave/verify/{tx_id}, with the existing per-IP IpRateLimiter used by
other cost-bearing routes. Preserve the current list_anchors and
verify_anchor_endpoint handlers while ensuring anonymous requests are throttled
before triggering gateway or database work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d81eba6b-bb9a-4ee3-af1e-5e62923f6b5f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/certs.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/main.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
beardthelion
left a comment
There was a problem hiding this comment.
Reviewed the current head against merged state. The certificate-chain design is genuinely strong and every serious finding from the earlier rounds is resolved on this head: per-ref cert binding, prev-hash linkage, fail-closed on a missing predecessor, atomic seq allocation (transaction-scoped advisory lock plus a (repo_id, seq) unique index and a 23505 retry), the gateway data-URL fix, the migration backfill-before-unique-index, and the full pusher-signature metadata on both cert endpoints. One functional bug blocks it: the verify endpoint never validates a real anchor. Findings highest first.
Findings
-
[P1] Compare the outer anchor repo against the certificate using the same identifier domain
crates/gitlawb-node/src/arweave.rs:334
The verify cross-check doesouter_repo != Some(&c.repo_id), but the outer anchor'srepois written as the slug{owner_key}/{name}(api/repos.rs:1222) while the embedded certificate'srepo_idis the repo UUID (issue_ref_certificate(&record.id, ...),api/repos.rs:1001). Slug never equals UUID, so every honestly produced anchor pushes a repo-mismatch error and returnsvalid: false. The endpoint cannot go green on real data: push to a public repo with a bundler configured, takearweave_tx_idfrom/api/v1/arweave/anchors, andGET /api/v1/arweave/verify/{tx_id}reports invalid despite a good node signature. This is the cross-check added in response to the earlier "verify does not compare the transition" finding, so the fix landed but across mismatched identifier domains; the ref/old/new/node comparisons beside it are correct. Either carry the UUID in the outer anchor, or resolve the slug to the repo id on the verify side before comparing. The verify tests set the embedded cert'srepo_idequal to the outer slug, which is why CI stays green while production never matches. -
[P2] Make the pusher authorization proof load-bearing, not silently skippable
crates/gitlawb-node/src/arweave.rs:465
The pusher-proof check is gated on all four ofpusher_sig,signature_input,content_digest,request_pathbeing present, but the node signing payload (cert.rs) covers onlypusher_sig— not the other three. A holder of a valid node signature can nullsignature_input/content_digest/request_path; the node signature still verifies (those fields are unsigned), the wholeif let (Some, Some, Some, Some)block is skipped, and verification returns valid with the independent RFC 9421 proof never checked. That defeats the stated goal of letting a third party verify the pusher authorization without trusting the node alone; it bites under node-key compromise. Bind the three context fields into the node payload and treat a presentpusher_sigwith missing context as invalid rather than passing. -
[P2] Bound the gateway body by bytes read, not Content-Length
crates/gitlawb-node/src/arweave.rs:293
The 1 MiB guard only short-circuits when the gateway sends an honestContent-Length; a chunked or header-omitting (or low-lying) response skips the pre-check, andresp.bytes().awaitthen buffers the whole body before the post-check runs. The verify route is unauthenticated (IP-rate-limited only) andtx_idis caller-chosen, so a permissionless caller can drive multi-hundred-MB allocations on the async worker, bounded only by the 10s client timeout. Stream with a running cap (resp.chunk()loop, abort past 1 MiB) rather than buffering first. -
[P2] Add an executed upgrade-path test for the v13 seq backfill
crates/gitlawb-node/src/db/mod.rs:923
The v13 backfill andidx_ref_certs_repo_seqbuild are never exercised throughrun_migrations()against pre-existing multi-cert data:v10_upgrade_dedup_via_migrationre-applies only v10 (it deletes just the v10 row fromschema_migrations), andmigration_v11_creates_owner_did_columnseeds no certificates. The backfill logic itself is sound (ROW_NUMBER() OVER (PARTITION BY repo_id ORDER BY issued_at, id)over a NOT NULL column with a total tiebreaker), but a data migration this consequential needs a test that seedsschema_migrationsat v12, inserts several same-repo/different-ref certs (allseq=1after v12), runs the migrations, and asserts distinctseqplus the unique index present. -
[P3] Cluster of smaller items
crates/gitlawb-node/src/arweave.rs:272
Config docs still point at the dead knob:.env.example, README, and thearweave.rsmodule comment referenceGITLAWB_IRYS_URLand never mentionGITLAWB_BUNDLER_URLorGITLAWB_ARWEAVE_GATEWAY(not a break —main.rsfalls back to the old env var with a deprecation warning — but the docs should match). A gateway-fetch failure or a malformed embeddednode_didreturns a 500 that echoes the internal error string (api/arweave.rs) rather than a cleanvalid:falsewith the right status.tx_idis unvalidated before being appended to the gateway URL (no host-swap SSRF given the fixed authority andredirect::none, but validate to the 43-char base64url shape as cheap defense).repo_lock_hashusesDefaultHasher, which the std docs do not guarantee stable across Rust versions, so two differently-built nodes on one Postgres could hash a repo to different lock keys (backstopped by the unique index and retry, so retry storms rather than corruption; prefer a stable hash). The outerold_sha/new_sha/node_didcross-checks are skipped when the field is absent (is_some()guards), unlike repo/ref; a forger who omits them still gets valid. Minor: the deadlock_repo_cert_issuancehelper locks on the pool connection (immediate release, a no-op) and should be removed or made a session lock, and the endpoint doc comment says "most recent local cert" while the code chains againstseq-1.
Core design is sound and the prior integrity findings are genuinely resolved; the P1 is the blocker and it is a last-mile identifier mismatch, not a redesign.
jatmn
left a comment
There was a problem hiding this comment.
Blocking findings
1. verify_anchor compares anchor repo slug against certificate repo_id
Severity: Blocking
Files: crates/gitlawb-node/src/arweave.rs:335, crates/gitlawb-node/src/api/repos.rs:1328, crates/gitlawb-node/src/cert.rs:97
The outer anchor payload stores the human-readable repo slug (owner_short/repo_name), while the embedded RefCertificate stores the database UUID (record.id). The verify cross-check does:
} else if outer_repo != Some(&c.repo_id) {For every real anchor these values differ, so the endpoint returns valid: false even when the node signature, prev hash, and pusher proof are correct. This makes the verify endpoint unusable on production data.
Fix: Either store repo_id in the anchor payload (or add a dedicated repo_id field), or resolve the slug to the repo UUID on the verify side before comparing. Prefer storing the UUID in the anchor and keeping the slug for display only.
2. gl cert show --verify uses the old 7-field signed payload
Severity: Blocking
Files: crates/gl/src/cert.rs:259-267, crates/gitlawb-node/src/cert.rs:25-36
The CLI reconstructs the node-signed payload with only the original seven fields:
{ "repo_id", "ref", "old", "new", "pusher", "node", "ts" }The node now signs ten fields:
{ "repo_id", "ref", "old", "new", "pusher", "node", "ts", "seq", "prev", "pusher_sig" }Because serde_json serializes maps alphabetically, the added keys change the signed bytes. gl cert show --verify will report INVALID for every certificate issued after this PR, even though the node and the Arweave verifier accept them.
The CLI tests payload_serialization_matches_frozen_canonical_form and verify_signature_round_trip_and_tamper still pin the old format and therefore pass while real-world verification fails.
Fix: Update verify_signature in gl/src/cert.rs to include seq, prev, and pusher_sig, and update the frozen canonical tests accordingly.
High-priority findings
3. New anchor lifecycle methods are dead code
Severity: High
Files: crates/gitlawb-node/src/db/mod.rs:2920-2975, crates/gitlawb-node/src/api/repos.rs:1342-1354
confirm_arweave_anchor, fail_arweave_anchor, and list_pending_anchors are marked #[allow(dead_code)] and never called. Anchors are inserted with status = 'pending', deadline_height = NULL, and receipt_sig = NULL, and no background worker ever updates them. The new status fields are therefore unreliable for monitoring.
Fix: Either add a background confirmation worker in this PR, or remove the unused columns/methods and defer the lifecycle feature to a follow-up.
4. record_arweave_anchor failures are silently dropped
Severity: High
File: crates/gitlawb-node/src/api/repos.rs:1342-1354
let _ = db_clone
.record_arweave_anchor(&crate::db::RecordAnchorInputV2 { ... })
.await;If the local DB insert fails, the anchor transaction exists on Arweave but is not recorded locally, with no log or metric.
Fix: Log a warning/error and optionally increment a metric when the insert fails.
5. contracts_info leaks URLs that may contain credentials
Severity: High
File: crates/gitlawb-node/src/server.rs:583-605
The unauthenticated /api/v1/contracts endpoint returns rpc_url, bundler_url, and arweave_gateway verbatim. If an operator configures a private RPC or paid bundler URL containing an API key, the key is exposed to anonymous callers.
Fix: Mask or omit URLs that may contain credentials, or require authentication for this endpoint.
6. Arweave verify route uses the per-DID creation limiter
Severity: High
Files: crates/gitlawb-node/src/server.rs:225-236, crates/gitlawb-node/src/main.rs:297-298
The verify route is throttled with state.rate_limiter, which is configured as a per-DID repo-creation limiter (10 requests per hour). Even though the middleware keys by IP, the threshold is far too restrictive for a public verification endpoint.
Fix: Add a dedicated Arweave IP rate limiter (e.g. GITLAWB_ARWEAVE_RATE_LIMIT) with a sensible default.
7. Historical certificates get broken prev values after migration
Severity: High
File: crates/gitlawb-node/src/db/mod.rs:923-950
Migration v13 renumbers seq but does not backfill prev. Existing rows except the first per repo keep the zero sentinel, so verify_anchor will reject all pre-upgrade anchors whose embedded certificate has seq > 1.
Fix: Backfill prev in v13 using the same canonical JSON + SHA-256 that cert::prev_hash uses, or explicitly document that historical anchors are intentionally unverifiable.
Medium-priority findings
8. Pusher proof is silently skipped when incomplete
Severity: Medium
File: crates/gitlawb-node/src/arweave.rs:465-551
verify_anchor only runs the RFC 9421 pusher verification when all four of pusher_sig, signature_input, content_digest, and request_path are present. If any is missing (e.g. pre-v13 certificates), the endpoint can still return valid: true without checking who authorized the push.
Fix: At minimum, emit an informational error/warning when a certificate is expected to carry a pusher proof but one is missing.
9. tx_id path parameter is not validated before gateway fetch
Severity: Medium
File: crates/gitlawb-node/src/arweave.rs:267
The user-supplied tx_id is appended directly to the gateway URL. Without length/format validation (Arweave IDs are 43-character base64url strings), the endpoint can be abused as a limited open proxy or SSRF vector against the configured gateway.
Fix: Validate tx_id against ^[A-Za-z0-9_-]{43}$ and return 400 Bad Request early.
10. Outer anchor checks are optional for old_sha, new_sha, node_did
Severity: Medium
File: crates/gitlawb-node/src/arweave.rs:351-371
The cross-check only errors if these fields are present and mismatch. If a malicious anchor omits them, verification still passes. They should be mandatory when a certificate is embedded.
Fix: Fail closed when old_sha, new_sha, or node_did are missing from the outer payload.
11. Advisory lock key uses unstable DefaultHasher
Severity: Medium
File: crates/gitlawb-node/src/db/mod.rs:2241-2246
repo_lock_hash derives the per-repo advisory lock key with std::collections::hash_map::DefaultHasher, which is not guaranteed stable across Rust versions or platforms. Different node builds could compute different lock keys and lose cross-instance serialization.
Fix: Use a stable hash such as the first 8 bytes of SHA-256(repo_id).
12. v1 schema was edited in-place
Severity: Medium
File: crates/gitlawb-node/src/db/mod.rs:462-473, :525-538, :648-663
The migration catalogue comment explicitly states that future changes must be added as new migrations and never appended to v1. However, v1 now already contains the columns that migrations v12/v13 add. While IF NOT EXISTS keeps upgrades safe, this breaks the migration narrative and can confuse future maintainers.
Fix: Either revert the v1 additions and rely solely on v12/v13, or document that v1 in this branch intentionally includes later columns.
Lower-priority findings
13. Test fixtures do not form valid certificate chains
Severity: Low-Medium
Files: crates/gitlawb-node/src/api/events.rs:429-454, crates/gitlawb-node/src/test_support.rs:4964-4997, crates/gitlawb-node/src/db/mod.rs:5215-5252
Test helpers use process-global sequence counters and hard-coded zero prev hashes. They do not exercise per-repo contiguous sequences or cryptographically correct prev linkage, so chain-related regressions could slip through.
Fix: Use per-repo sequence allocation and compute prev from the predecessor certificate, matching production issuance.
14. local_cert events omit the new certificate fields
Severity: Low-Medium
File: crates/gitlawb-node/src/api/events.rs:243-256
The event feed does not include seq, prev, pusher_sig, signature_input, content_digest, or request_path, so event consumers cannot validate chain continuity or reconstruct the pusher proof.
Fix: Include the new fields in the event payload.
beardthelion
left a comment
There was a problem hiding this comment.
Traced the signing/verification payload construction on both the node and gl sides line-by-line, and cross-checked the migration runner and the pre-existing rate-limiter/auth-middleware code this PR reuses. Most of the design (append-only per-repo hash chain, advisory-lock-serialized seq allocation, streamed-and-capped gateway fetch, fail-closed cross-checks) is solid. One finding should block merge.
Findings
-
[P1] gl cert show always reports a legitimate push-issued certificate as invalid
crates/gl/src/cert.rs:265
The node signs the certificate over 13 fields (crates/gitlawb-node/src/cert.rscert_payload), includingsignature_input,content_digest, andrequest_path.gl's client-sideverify_signaturerebuilds the payload to check the signature against but stops atpusher_sig, never reading or including those three fields.git_receive_packalways passesSome(..)for all three (the route sits behindrequire_signature, which unconditionally sets them), so every certificate issued for a real push carries them, and the server API (api/certs.rs) already returns them in the JSON —cmd_showjust never parses or forwards them. The byte mismatch means the Ed25519 checkgl cert showruns fails for every push-issued certificate, reporting a validly node-signed cert as tampered. The PR's owngl/src/cert.rstests don't catch this because they only exercise the old 10-field shape (pusher_sig: null, no context fields).
Fix: addsignature_input,content_digest,request_pathtogl'sverify_signaturepayload and its call sites, matching the server'scert_payloadexactly, then add a test that signs and verifies a cert with all three fields populated. -
[P3] .env.example and README still document the renamed config knob
.env.example,README.md
Both still referenceGITLAWB_IRYS_URLonly; neither mentions the newGITLAWB_BUNDLER_URLorGITLAWB_ARWEAVE_GATEWAY(config.rs).main.rsdoes fall back to the legacy env var with a deprecation warning, so this isn't a functional break, just stale operator-facing docs for a knob this PR renamed. -
[P3] verify_anchor 500s on a malformed node_did instead of returning valid:false
crates/gitlawb-node/src/arweave.rs
Every other malformed-input case inverify_anchor(gateway non-2xx, oversized/undecodable body, non-JSON payload) returnsOk(VerifyResult{valid:false, ..}). The node-DID parse (gitlawb_core::did::Did::from_str(&c.node_did).map_err(..)?) still uses?, so a certificate whose embeddednode_didfails to parse propagates asErr, which the handler turns into a 500 instead of the same controlled{valid:false}response every other bad-input path returns.
beardthelion
left a comment
There was a problem hiding this comment.
Re-reviewed at 0b7a4f08. Merge-base matches origin/main, CI is 12/12 green on this head, and the rebase ask from the last round is satisfied. I read all four comment surfaces: seven inline CodeRabbit threads, all resolved and outdated; jatmn's open round is still CHANGES_REQUESTED on 0f8c7f26, with no new human review on this push yet. The certificate-chain core from earlier rounds still holds (per-ref certs, issuer pinning, fail-closed corroboration, streaming 1 MiB cap on verify). I ran anchor_record_failure_is_reconciled_without_double_pay green on this head.
The rebased integration did not close the gateway-binding gaps or the Pinata coupling on the durable path. Five ranked open PRs overlap repos.rs, auth/mod.rs, and db/mod.rs on this branch; expect more conflict churn if those land first, but the findings below are about correctness on this head, not merge order.
Findings
-
[P2] Bind
verify_anchorgateway bodies to the requestedtx_id
crates/gitlawb-node/src/arweave.rs:711verify_anchorfetchesGET {gateway}/{tx_id}but only JSON-parses the body and runs cert/identity checks. It never comparestx_idtoans104::data_item_id(body_bytes)or any payload id field before returningvalid: true. A gateway or proxy that returns the same valid anchor JSON for every id can certify the wrong transaction.anchor_item_present's raw-bytes path already binds ids; apply the same binding here (schema gate plus id match), and add a RED test where the mock returns identical JSON forwrong-txandright-tx. -
[P2] Bind the JSON recovery probe to the probed
item_id
crates/gitlawb-node/src/arweave.rs:524On a JSON 200,
anchor_item_presentreturnsOk(true)whenschema == gitlawb/ref-update/v1without checking the response matches the requesteditem_id. The raw-bytes path derivesdata_item_idand compares (:533-545). A generic gitlawb-shaped JSON 200 can make crash recovery treat an uncertain upload as landed and skip a needed re-upload. Require id binding on the JSON path too, or fall through to the bytes check and fail closed on mismatch. -
[P2] Keep best-effort Pinata work out of the durable anchor completion path
crates/gitlawb-node/src/api/repos.rs:2674run_post_receive_jobawaitsanchor_cid_rxbeforeanchor_ref_updates, but the Ok sender lives inside the detached Pinata task and runs only afterpin_sem_pinata.acquire_owned().awaitandpinata::pin_new_objectscomplete (:3371-3427). Comments mark Pinata as best-effort, yet the durable job blocks on it. When the global pin semaphore is saturated, the job staysprocessingand anchor rows never reachrecordeduntil restart retries the same dependency. CID is alreadyOptioninanchor_ref_updates; report the announce decision (and any locally available CID) before dispatching Pinata, or treat CID enrichment as a separate best-effort update. Add an isolation test that holds the pin semaphore and proves the job reaches terminal anchor state without waiting for Pinata. -
[P2] Add a load-bearing test for the per-transition anchor claim
crates/gitlawb-node/src/db/mod.rs:4473two_concurrent_workers_claim_the_job_onceracesclaim_post_receive_jobonly. TheON CONFLICT (repo, ref_name, old_sha, new_sha) DO NOTHINGguard at:4473is real code but not pinned by a test that races two workers at the anchor-claim layer. Add a test where two tasks reachclaim_anchor_claimconcurrently and assert exactly one upload path wins.
Not an ask, recorded only: verify_anchor still accepts JSON gateway bodies only; raw ANS-104 bytes at GET /{tx_id} fail with "not valid JSON" even when authentic. Worth aligning with anchor_item_present's dual representation handling, but secondary to the id-binding gaps above.
One process note, not a finding: this branch overlaps #134 on server.rs / arweave.rs and several ranked security PRs on auth/repo paths. Rebase conflict resolution on those surfaces will need another pass when those PRs land.
Remove stale upstream code blocks concatenated by rebase conflict resolution (inline record_push/issue_ref_certificate, duplicate guard.release/result handling, dead standalone sweep_rate_limiters). Fix EncryptTaskCtx missing fields, repo_store local_path visibility, git_receive_pack test arg order, and f2a_tail missing ref_certs arg.
0b7a4f0 to
aa9fc8b
Compare
beardthelion
left a comment
There was a problem hiding this comment.
Re-reviewed at aa9fc8b2. CI on that head is 10/12 green: fmt + clippy, MSRV, audit, Docker, and release jobs pass; test (stable) and test (beta) fail before any test logic runs because migration v26 aborts (gh run 33011515250). jatmn's last human review is on 6bf13d1e (2026-08-15); the commits since then address most of that round (explicit gateway validation, synchronous anchor outbox in the job body, README bundler-token contract, verify_anchor id binding). I am not re-litigating those as open on this head.
Findings
-
[P1] Strip the cert-chain DDL out of migration v26
crates/gitlawb-node/src/db/mod.rs:1168Migration v26 (
pin_repair_sweep_discovery_cursor) currently runs anUPDATE ref_certificates SET seq = ..., createsidx_ref_certs_repo_seq, and addssignature_input/content_digest/request_path. Those statements belong in v32 (append_only_certs_and_pusher_proof), after v29 addsseq. On a fresh database v26 runs beforeseqexists, so every#[sqlx::test]panics attest_support.rs:43withcolumn "seq" of relation "ref_certificates" does not exist. I confirmed againstorigin/main, where v26 only adds the twopin_repair_sweepdiscovery columns. The duplicate block atdb/mod.rs:1200-1231is rebase debris fromaa9fc8b2; remove it from v26 and keep the cert-chain work solely in v32. -
[P2] Add a migration-order regression test that would have caught v26
crates/gitlawb-node/src/db/mod.rs:8099v26_discovery_continuation_applies_on_upgradeonly checks the pin-repair discovery columns. It does not exercise a full fresh-chain apply through v32, which is the path CI runs. After fixing v26, add a test that applies migrations v1 through v32 on an empty database (or extend the existing replay upgrade test atdb/mod.rs:10004) so cert-chain DDL cannot land before theseqcolumn again.
jatmn's Aug 15 gateway-inference, detached-tail durability, README bundler-token, and upload-without-DB-record findings look addressed on this head: gateway inference is gone from main.rs, Config::validate() requires an explicit gateway when a bundler is set, run_post_receive_job awaits anchor_ref_updates with the outbox pattern, and README matches config.rs on the token slug. I am not restating those as open asks.
Not an ask, recorded only: gl cert show still prints VALID on internally consistent certs without --verify (issuer not anchored to the queried node). verify_anchor skips prev-chain checks for legacy zero-prev rows and warns when outer repo/owner_did are omitted and the repo is absent from the local DB. Both match documented tradeoffs; operators should use gl cert show --verify for third-party checks.
One process note, not a finding: expect a rebase conflict with #134 on arweave.rs if both land close together. This PR also overlaps repos.rs, db/mod.rs, and auth/mod.rs with several in-flight security PRs (#285, #306, #325, #314, #382); merge order may require a follow-up pass on merged state.
- Restore v19 (pinned_cids_repo_provenance) dropped by rebase concat - Remove stale cert-chain stmts from v26 (belongs in v32 only) - Add ESCAPE '!' to list_ref_certificates_by_prefix LIKE query - Add arweave_rate_limiter to AppState sweep - Move post-receive job spawn above guard.release for disconnect safety - Add PusherSignature/PusherProof to signed_request_as test helper
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Make the post-receive outbox durable before the ref update can become externally successful
crates/gitlawb-node/src/api/repos.rs:2258
crates/gitlawb-node/src/api/repos.rs:2321
smart_http::receive_pack completes before this handler constructs and persists post_receive_jobs. At that point Git has already advanced the ref on disk, while the only recovery mechanism added by this PR still exists solely in the request future. A process crash, shutdown, or database failure between those two operations leaves an accepted transition with no job for drain_post_receive_jobs to enumerate. That loses the promised push accounting, trust update, per-ref certificate, and durable anchor for the original transition; sending the client an error cannot undo the landed ref, and retrying can be a no-op or represent a different transition.Please address the root atomicity/recovery boundary rather than adding another retry around enqueue_post_receive_job: ensure every ref transition that Git can expose has a durable, deterministic recovery record, or add startup reconciliation that derives and queues exactly the missing work for already-landed transitions. The solution must preserve the existing exactly-once properties across restart and concurrency: one accounting event, deterministic per-job/per-ref certificates, and no duplicate paid anchor. Add fault-injection coverage for a successful receive-pack followed by failed job persistence, then restart/drain and assert the original transition is recovered exactly once.
-
[P1] Authenticate the ANS-104 envelope before treating the embedded certificate as an anchor
crates/gitlawb-node/src/arweave.rs:694
crates/gitlawb-node/src/ans104.rs:392
verify_anchor binds the response bytes to the requested item ID and calls data_item_data, but that helper is intentionally structural-only: it does not validate the ANS-104 Ed25519 signature, its deep-hash, or the envelope owner. The subsequent certificate verification proves that a copied certificate was signed by this node; it does not prove the permanent data item containing that certificate was authored by this node. An attacker can create a syntactically valid item under their own key, embed a copied valid certificate and matching outer fields, and have the verify endpoint report it as valid.Please make envelope authenticity an explicit prerequisite of payload parsing/adjudication. Recompute the ANS-104 signature preimage from the served item, verify it with the configured node key rather than a key supplied by the artifact, and reject owner, signature, or deep-hash mismatches before accepting any JSON or certificate. Keep the certificate-version fallbacks for historical certificates separate from envelope authentication: old certificate payloads may remain supported, but every newly claimed signed anchor still needs a valid node-authored ANS-104 wrapper. Add a positive trusted-envelope test plus rejection tests for a tampered signature, a foreign owner, and a forged envelope containing an otherwise valid copied certificate.
Review guidance
This PR has accumulated follow-up findings because it joins several security-sensitive state machines: Git ref visibility, durable database recovery, certificate-chain provenance, paid immutable uploads, and public verification. Each layer is individually careful, but correctness depends on the boundaries between them. In both findings, the code validates or persists a later layer after an earlier irreversible effect has already occurred: Git makes a ref durable before the recovery record exists, and the verifier trusts an inner certificate before authenticating the enclosing immutable artifact.
For the remaining revision, please drive the implementation from the end-to-end invariants rather than from individual error branches:
- For every accepted ref transition, define the durable identifier and recovery record before that transition can be acknowledged or otherwise become visible; test crash/restart at each boundary through accounting, certificate issuance, and anchoring.
- For every verifier verdict, establish the trust anchor outside the fetched artifact first, authenticate each enclosing format from outside-in, and only then interpret nested claims.
- Exercise fault-injection and adversarial tests that remove or invert each boundary. A happy-path upload, a valid inner certificate, or a passing retry test is not sufficient proof that a crash or substituted remote artifact cannot bypass the contract.
Keeping those invariants explicit should reduce further review churn while preserving the intended scope: durable per-transition bookkeeping and trustworthy signed Arweave anchors, without broadening replication behavior or redesigning the certificate protocol.
beardthelion
left a comment
There was a problem hiding this comment.
I re-read head 04271ad against merged state. CI is green on this push. The R4/R5 work closes the signed-tuple legacy corroboration hole, binds verify to the served ANS-104 item id, requires Content-Digest before certificate proof, masks gateway credentials on the anchors listing, and adds real accept-path verify tests. Two verify/recovery gaps are still open.
Findings
-
[P2] Verify the ANS-104 data-item signature before reporting valid
crates/gitlawb-node/src/arweave.rs:705
Upload signs ref anchors with the node keypair (build_ref_anchor_item, line 139) and the mock bundler enforcesans104::verify_data_item.verify_anchoronly checks thatdata_item_id(body) == tx_id, unwraps the payload, and validates the embedded certificate. It never checks that the item's Ed25519 signature or owner matchesnode_did. The accept tests wrap payloads withserve_signed_anchor(..., &node_kp, ...), so they pass without exercising a foreign signer. Anyone can republish a legitimately node-signed certificate inside their own ANS-104 item and getvalid: trueon their tx id without this node having uploaded it. -
[P2] Cap the gateway presence probe body the same way verify caps its fetch
crates/gitlawb-node/src/arweave.rs:507
verify_anchorstreams the gateway response with a 1 MiB running cap (lines 661-690).anchor_item_presentcallsresp.bytes().awaitwith no size limit before it parses the item id. Post-receive recovery uses that probe to decide whether a crashed upload landed before re-paying. A hostile or misconfigured gateway can force an unbounded allocation on the worker path while verify itself is bounded. -
[P3] Return 400 for a malformed
?repo=filter on the anchors listing
crates/gitlawb-node/src/db/mod.rs:4748
R3 added rejection for malformed repo filters, but the error is a bareanyhow!that propagates through the handler as 500 Internal. Map that shape toBadRequestat the handler boundary so clients get a 400, not an internal error.
Not an ask, recorded only: legacy migration rows with the all-zero default prev still skip chain linkage with a warn only; signed-tuple corroboration blocks seq tamper when a matching stored row exists. Encrypted-manifest anchoring on the replication tail still lacks the UploadOutcome / probe reconciliation the durable ref path has; the PR scope treats that tail as best-effort.
One process note, not a finding: this branch overlaps open PRs on auth/mod.rs (#306) and repos.rs (#285, #325). Base is current main on this head, but landing those first may still need a conflict pass on the shared hunks.
…velope auth, gateway probe cap, ?repo= 400 Twigpine#26 review found four gaps in the durable Arweave anchoring path. Two reviewers in two rounds; this commit closes every one. **P1 durability: pending_ref_transitions outbox + startup recovery** A push that crashed between the bare-repo ref advancing and the post_receive_jobs row landing left an accepted ref with no recovery record. The handler now writes one row to pending_ref_transitions per ref-update on the same success path that lands the ref, BEFORE calling enqueue_post_receive_job. On startup, recover_orphan_ref_transitions (called from drain_post_receive_jobs BEFORE the existing reset/drain pass) derives a deterministic-id post-receive job per orphan. The deterministic id makes a second recovery a no-op via the existing ON CONFLICT (id) DO NOTHING enqueue; the anchor claim layer is unique on (repo, ref_name, old_sha, new_sha), so no duplicate paid anchor. The recovery uses PostReceiveAttestation::default() (all None) so the resulting cert is 7-field legacy form. The original RFC 9421 proof fields cannot be regenerated from on-disk state. Such certs are accepted by the verifier's 7-field fallback and chained to their predecessor via the per-repo_id seq stored at issuance. The residual is documented in the docstrings of enqueue_post_receive_job, recover_orphan_ref_transitions, and verify_anchor (durable doc per team memory). Migration v33 (pending_ref_transitions) is additive. v32 remains the last migration on this branch. **P1/P2 envelope authentication: ANS-104 signature gate in verify_anchor** verify_anchor now takes node_vk: &ed25519_dalek::VerifyingKey and authenticates the served body against it before parsing the embedded cert. A cert copied into a foreign envelope (signed by a third party whose data_item_id still matches the requested tx_id because the id derives from the signature region) used to return valid: true; it now returns valid: false with the same "refuses to adjudicate" shape the existing id-mismatch and bare-JSON paths use. Legacy 7/13/14-field certs already on the network continue to verify under their respective payload forms — the envelope check is the gate for newly claimed signed anchors only. verify_data_item and the deserialize_tags / read_string / read_long helpers it calls are no longer cfg(test); they were production-grade helpers with a historical test-only gate. deserialize_tags now short-circuits on an empty buffer (the production bug the test gate was hiding). **P2 gateway probe cap: read_capped_body helper** anchor_item_present used to call resp.bytes().await with no size cap while verify_anchor already streamed with a 1 MiB cap. A hostile gateway could drive unbounded allocation on the recovery path. read_capped_body is a private helper that returns the body or a credential-safe error text within a per-call limit. verify_anchor keeps the existing 1 MiB cap and on overflow returns the existing populated VerifyResult shape. anchor_item_present uses 65_536 and on overflow returns Ok(false) — a 2xx with a body too large to be a real ANS-104 item cannot be a real data item, and the durable job must re-upload rather than fail-closed. **P3 handler error mapping: 400 not 500 for ?repo=** list_anchors now validates the same owner/name shape the DB layer rejects and returns AppError::BadRequest (400). The DB-layer rejection stays as defense-in-depth so any other internal caller cannot accidentally turn a malformed filter into an unfiltered listing. **Tests (8 new)** - test_verify_anchor_envelope_matching_node_signer_adjudicates - test_verify_anchor_envelope_with_tampered_signature_is_invalid - test_verify_anchor_envelope_signed_by_foreign_key_is_invalid - test_verify_anchor_envelope_with_copied_cert_claiming_foreign_node_did_is_invalid - probe_caps_oversized_response_body - list_anchors_malformed_repo_returns_400 - post_receive_job_survives_persist_failure - post_receive_recovery_is_idempotent_under_double_drain All 1160 unit tests + 7 integration tests pass. cargo clippy --all-targets -D warnings clean.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jatmn
left a comment
There was a problem hiding this comment.
This PR has grown beyond a reviewable and safely mergeable scope. Please stop revising it as one branch. Split the work into the independent PRs described below and treat this PR as superseded; another monolithic follow-up will recreate the same review and integration churn.
Required next step: split the PR
Create four narrow, self-contained PRs:
-
Durable post-receive lifecycle
Own only the Git transition intent/outbox, authentic pusher and RFC 9421 proof persistence, restart drain, push accounting, certificate issuance, and anchor handoff. This PR must close the pre-outbox crash boundary and prove exactly-once recovery. Do not include ANS-104 format work, public gateway/API changes, policy documentation, or unrelated migrations.
-
Arweave transport and verification
Own only ANS-104 serialization and verification, bundler/gateway request-response binding, recovery outcome classification, URL and credential handling, and the public verification endpoint. Model recovery as three outcomes—
present,definitively absent, andindeterminate—and permit another paid upload only for a trustworthy, protocol-defined absence. -
Certificate, API, and CLI compatibility
Own only certificate payload versions, API representation, CLI display and verification, and legacy compatibility fixtures. New fields must be optional for readers, and both old-server/new-client and new-server/old-client combinations must remain safe. None of the three current findings is assigned to this PR; it is still a required split because this protocol/client work is independently reviewable and should not remain coupled to post-receive recovery.
-
Configuration and operator policy
Own only configuration validation/default tests and the corresponding
.env.example, README, and SECURITY.md changes. This PR should make every operator-facing surface describe the tested runtime behavior consistently; it should not carry unrelated runtime changes.
Independence and overlap requirements
Every split PR must be safe to land on current main without any sibling PR:
- It must compile, migrate, run, and pass its focused tests by itself. Do not require an unmerged branch, a coordinated same-day merge, hidden deployment steps, or manual database repair.
- If it reads a new table or column, include the required append-only migration and upgrade test in that same PR. Do not edit released migrations. Reserve distinct migration versions so the final schema is the same regardless of which PR lands first.
- If it changes a serialized payload or API response, version the boundary or use optional fields and compatibility fallbacks. Do not introduce a shared signature or wire-format change that requires another PR to land simultaneously.
- State one owned invariant in the PR description and list the producer, persistence, restart/restore, consumer/effect, and cleanup/failure boundaries covered by its tests.
There is also active work on the same contracts. Before opening each split PR, explicitly state whether it supersedes, excludes, or composes with the following open PRs:
- #134: authentication for the anchors route
- #285: receive-pack/advisory-lock behavior
- #306: signed requests and
Content-Digest - #314: DID/Ed25519 identity validation
- #324: node/libp2p identity handling
- #325: authenticated ref-update ingestion
- #382: replication and withheld-object pinning
These overlaps are semantic, not merely textual: they affect authorization, request provenance, receive-pack execution, identity, and replication. Choose one owner for each contract. Do not carry competing implementations into another broad conflict-resolution branch.
Findings the split PRs must address
-
[P1][Split PR 1] Persist authentic recovery state before a successful Git update can be lost
crates/gitlawb-node/src/api/repos.rs:2258receive_packcan apply the ref before the handler reaches the firstrecord_pending_ref_transitioncall at line 2341. If the process exits, the request is cancelled, or that DB write fails in this interval, neitherpending_ref_transitionsnorpost_receive_jobsrecords the landed transition. Startup drain enumerates only those sources, so it cannot reconstruct the missing work. Returning an error cannot roll back the Git ref: the original push permanently lacks accounting, its per-ref certificate, and its required anchor.The partial fallback after a transition row exists does not preserve provenance either. Because that row stores no authenticated pusher or RFC 9421 proof, recovery substitutes
did:key:recoveredand an empty attestation, then emits a normal node-signed legacy certificate. That is not equivalent to recovering the original authenticated push.Address the irreversible-effect boundary itself. Before Git can leave a visible ref with no durable record, persist enough authentic intent to identify and recover the transition, or implement reliable startup reconciliation that can derive every landed-but-unaccounted transition without inventing its identity. Preserve exactly-once accounting, certificate issuance, and anchor payment across retries. A retry added after
receive_packdoes not close this gap.Required proof: inject failure after Git applies the ref but before the first transition/job write, restart the node, and show that the original transition produces exactly one push event, one certificate carrying the original pusher/proof, and at most one anchor upload. Also prove that a failed or cancelled receive-pack does not turn a prepared intent into completed accounting or anchoring.
-
[P1][Split PR 2] Do not treat gateway 400/410 responses as proof that an uncertain upload is absent
crates/gitlawb-node/src/arweave.rs:546anchor_item_presentmapsBAD_REQUESTandGONEtoOk(false).anchor_ref_updatesinterprets false as permission to reclaim the row and upload again. Neither response proves that the previous paid item was never accepted: 410 can describe an artifact that existed but is no longer served, and 400 can be produced by gateway, proxy, routing, or request-processing failure.The concrete failure sequence is: the bundler accepts the item, the response or terminal DB write is lost, recovery probes the persisted
item_id, the gateway returns 400 or 410, and the node pays for a second immutable item for the same transition. This contradicts the PR's fail-closed, no-double-payment recovery contract.Fix the classification at its source. Only a trustworthy, protocol-defined absence may authorize another upload. Transport failures, malformed responses, item-ID mismatches, 400, 410, and other ambiguous outcomes must keep the outbox non-terminal and retry the probe without paying.
Required proof: for a persisted
item_id, cover confirmed presence, confirmed absence, 400, 410, transport failure, malformed or oversized response, and a 2xx response bound to a different item. Assert that only confirmed absence sends another upload request. -
[P2][Split PR 4] Make SECURITY.md state the actual owner-push default
SECURITY.md:67SECURITY.md states twice that
GITLAWB_ENFORCE_OWNER_PUSHdefaults tofalse, whileConfigdefaults it totrueand the README and environment example agree with the runtime. The document therefore tells operators that any authenticated DID may push unless they enable the flag—the inverse of the live authorization policy. This PR also adds the second incorrect limitation while claiming the security policy was aligned with runtime.Use the tested configuration default as the source of truth. State that owner enforcement is enabled by default and describe
GITLAWB_ENFORCE_OWNER_PUSH=falseonly as the explicit rolling-upgrade compatibility override. Consolidate the duplicate limitation sections.Required proof: add a small regression check that keeps CLI parsing, environment behavior,
.env.example, README, and SECURITY.md aligned for security-sensitive defaults.
Why the current approach keeps producing follow-up findings
The remaining defects are not isolated local mistakes. This branch combines multiple irreversible and externally visible state machines—Git ref application, database recovery, signed certificate provenance, paid immutable uploads, gateway reads, public APIs, and operator security policy. The failures occur at the handoffs between them:
- idempotence is expected to recover a transition that was never durably recorded;
- a remote response is reduced to a boolean even when its semantics are uncertain;
- tests start from already-seeded recovery state instead of failing at the irreversible boundary;
- runtime defaults and operator documentation have separate sources of truth;
- parallel security PRs modify the same contracts while this branch continues to absorb conflict resolutions.
The split above is intended to make each invariant finite and reviewable. For each new PR, include a compact state-transition table or failure matrix and make the tests load-bearing: removing the durable intent, changing an indeterminate probe result to “absent,” or restoring the incorrect security default must make the corresponding test fail. That evidence will show the lifecycle is closed instead of demonstrating only that the latest reported branch was patched.
Superseded by re-review at 193cfa5
beardthelion
left a comment
There was a problem hiding this comment.
Re-reviewed head 193cfa5f. CI is 12/12 green. I ran anchor_record_failure_is_reconciled_without_double_pay (pass) and gutted ans104::verify_data_item in verify_anchor (foreign-envelope test goes RED). Jatmn's new round on this head asks for a four-PR split; I agree. This branch still couples Git ref application, recovery outbox, paid immutable uploads, and public verify in one diff, and the defects below sit at those handoffs.
Findings
- [P1] Persist authentic transition intent before
receive_packcan leave a durable ref
crates/gitlawb-node/src/api/repos.rs:2258
smart_http::receive_pack runs at line 2258; the first record_pending_ref_transition call is at line 2341, after Git may have advanced the ref on disk. A crash, cancellation, or DB failure in that window leaves no outbox row and no post_receive_jobs row, while startup drain only replays those sources. The recovery fallback that invents did:key:recovered does not restore the original pusher or RFC 9421 proof. Persist enough authentic intent before the irreversible Git effect, or reconcile every landed-but-unaccounted transition without fabricating identity.
- [P1] Do not treat gateway 400/410 as proof the probed item is absent
crates/gitlawb-node/src/arweave.rs:546
anchor_item_present maps BAD_REQUEST and GONE to Ok(false). Recovery then reclaims the outbox row and pays for another upload. Neither status proves the prior paid item never landed: 410 can mean "no longer served" and 400 can be routing or proxy failure. Only a trustworthy, protocol-defined absence should authorize another upload; ambiguous outcomes must keep the row non-terminal and retry the probe without paying.
- [P2] Fix SECURITY.md to match the runtime owner-push default
SECURITY.md:67
SECURITY.md states twice that GITLAWB_ENFORCE_OWNER_PUSH defaults to false. Config sets default_value_t = true at config.rs:98, and README and .env.example match the runtime. Operators reading SECURITY.md will think any authenticated DID may push until they opt in, which is the inverse of production behavior.
- [P2] Stop logging
bundler_tokenon successful uploads
crates/gitlawb-node/src/arweave.rs:286
Success paths log bundler_token = %bundler_token at lines 286 and 412. That value is the Irys payment-token slug billed at /tx/{token}, not a display label. Error paths redact credentials; info-level success logs would still park a live payment credential in centralized tracing.
One process note, not a finding: split PRs need explicit composition with open #134, #285, #306, #314, #325, and #382 on the same contracts (auth, receive-pack, identity, replication). This branch overlaps those files; do not resolve conflicts by absorbing competing implementations into another monolithic branch.
Not an ask, recorded only: encrypted-manifest anchoring on the replication tail still lacks the ref-anchor outbox and probe reconciliation; the PR treats that path as best-effort.
|
Closing in favor of a four-way split. Review feedback was that the branch had grown beyond a reviewable scope: durable post-receive lifecycle, Arweave transport/verification, certificate/CLI compatibility, and operator policy were all coupled together, and that's why each round kept producing new findings at the handoffs. Replacing this PR with four narrow, independently mergeable splits, each from
Tracking the new branches in the team's issue-26 thread. Thanks for the detailed review — splitting is the right call. |
… 2/4) (Twigpine#524) * fix(node): ANS-104 transport, three-outcome probe, verify endpoint (Twigpine#26 split 2/4) Reviewer 2 closed PR Twigpine#224 on 2026-08-28 with a directive: split into four narrow PRs. This is Split PR 2 (Arweave transport and verification). The P1 finding the reviewer assigned to this split: the legacy `anchor_item_present` mapped `BAD_REQUEST` and `GONE` to `Ok(false)`, and the recovery code interpreted false as permission to reclaim the row and pay for another upload. Neither 400 nor 410 proves a previously paid item was never accepted: 410 can describe an artifact that existed but is no longer served, and 400 can be produced by gateway, proxy, or routing failure. The concrete failure sequence the reviewer named: bundler accepts the item, the response or terminal DB write is lost, recovery probes the persisted item_id, the gateway returns 400 or 410, and the node pays for a second immutable item for the same transition. Fix: model the probe result as three outcomes, and authorize a re-upload only on a trustworthy, protocol-defined absence. NEW MODULE crates/gitlawb-node/src/ans104.rs - DataItem: the on-wire ANS-104 shape (signature, owner, target, anchor, tags, data — all base64url-encoded except target/anchor). - deep_hash: the canonical signing input per the spec. The signature is over a SHA-256 mix of the dataitem/list/map discriminators, the signature type, owner, target, anchor, the recursively-hashed tag list, and the data payload. A hand-rolled sha256 of the JSON body would produce a hash no Arweave gateway would recognize; the deep-hash is the only correct form. - sign_data_item: Ed25519 over deep_hash. - verify_data_item: parses the owner, checks the signature against an expected pubkey, and reports a specific failure reason on each branch. - 7 unit tests covering round-trip, flipped signature, wrong expected key, mutated data, wire-shape round-trip, deep-hash stability, and the empty-tags edge case. NEW MODULE crates/gitlawb-node/src/arweave_v2.rs - ProbeOutcome: Present (2xx, body parses as ANS-104, sig verifies), DefinitivelyAbsent (404 with a known JSON body shape), Indeterminate (everything else). The reviewer's three-outcome model. - ProbeOutcome::permits_reupload: the recovery policy. ONLY DefinitivelyAbsent authorizes a paid re-upload. Indeterminate keeps the outbox non-terminal; Present skips re-payment. - probe_anchor_item: the gateway probe. 2xx bound to a different item id is Indeterminate, not Present. 2xx with a body that does not parse as ANS-104 is Indeterminate. Oversized bodies are Indeterminate (defense against body-stuffing). - read_capped_body: enforces PROBE_MAX_BODY_BYTES (1 MiB) on the full body, with a Content-Length fast-path that rejects before reading. - verify_anchor: the full path the public verify endpoint takes. Fetches the data item, parses it, verifies the Ed25519 signature against the persisted node_did, decodes the data payload as JSON, and reports the result. - 15 unit tests pinning each classification boundary: 400/410/5xx/transport/2xx-non-json/2xx-bad-sig/2xx-different- owner are all Indeterminate; 404 with empty or known-JSON body is DefinitivelyAbsent; 2xx with a valid signed item is Present; the re-upload policy is exhaustive. NEW DB METHOD crates/gitlawb-node/src/db/mod.rs - get_arweave_anchor_by_item_id: looks up an existing arweave_anchors row by its id (which is the Irys tx id, the same value the Arweave gateway uses to serve the data item). Used by the public verify endpoint to fetch the persisted node_did so the envelope signature can be verified against it. NEW CONFIG crates/gitlawb-node/src/config.rs - GITLAWB_ARWEAVE_GATEWAY_URL: the Arweave gateway the probe reads from. Defaults to https://arweave.net; operators can point it at a private mirror. NEW HTTP ENDPOINT crates/gitlawb-node/src/api/arweave.rs - GET /api/v1/arweave/anchors/verify/{item_id}: the public verify endpoint. Returns {item_id, status, verified, owner_did, data_payload, error} where status is one of "verified" / "definitively_absent" / "indeterminate". - 3 endpoint-level tests covering the verified, 404, and 400 shapes via the live router, asserting the public surface surfaces the three outcomes. WIRE-UP crates/gitlawb-node/src/server.rs - The verify route is registered alongside the existing /api/v1/arweave/anchors list route. NOT IN THIS SLICE (the bundler upload, the next commit): - The handler that builds a DataItem from a ref-cert and POSTs it to the bundler is not in this PR. The ANS-104 module is ready to be called by it; the cert/push-event flow in PR 1 persists the row that the bundler upload would consume. - PR 1's recovery drain produces an `anchor_jobs` row keyed on the (repo, ref, old, new) tuple; the next slice reads that row, builds the ANS-104 data item, POSTs to the bundler, and writes the resulting tx id to arweave_anchors. Compiles clean, 1113 tests pass with 0 regressions, clippy clean under -D warnings, fmt clean. Cross-PR overlap (declared in the PR description): - Twigpine#134 (anchors auth): composes. The verify endpoint reads from arweave_anchors; Twigpine#134's auth layer applies unchanged. The probe model is server-side and unauthenticated by design. - Twigpine#285 (advisory-lock session affinity): independent. The probe is read-only. - Twigpine#306 (Content-Digest on signed requests): independent. The probe does not sign requests. - Twigpine#314 (small-order Ed25519): composes. The verify path calls Did::to_verifying_key which already enforces the small-order check from Twigpine#314. - Twigpine#324 (libp2p keypair persistence): independent. - Twigpine#325 (gossip ref-update auth): independent. The verify path is for the public Arweave gateway, not gossip. - Twigpine#382 (replication withheld-subtree trees): independent. * fix(node): address reviewer findings on Twigpine#26 split 2/4 (transport) - P1 (DB lookup): get_arweave_anchor_by_item_id now filters on irys_tx_id (the externally-routable item id) rather than the internal UUID primary key. The production writer stores a fresh UUID in `id` and the Irys response / ANS-104 derived id in `irys_tx_id`; the old filter would always 404. New integration tests exercise the production writer path with distinct values for the two columns. - P1 (verify-gate): verify_anchor now gates on repo read via authorize_repo_read. The route is split in server.rs: the public list_anchors stays ungated (Twigpine#134 tracks that surface), the verify route gets optional_signature per the team memory `axum-layer-vs-merge-pitfall.md`. Denials on row-lookup, gate, and missing repo all collapse to the same opaque 404 so the public endpoint does not leak anchor-row existence. - P1 (ANS-104 deep-hash): rewritten to the verified SHA-384 recursive list/blob algorithm (matches `arbundles` JS reference). deep_hash returns [u8; 48]; signature is over the raw 48-byte digest. Three reference vectors from `Irys-xyz/arbundles/src/__tests__/deepHash.spec.ts` are bit-exact-asserted as the interop canary per the team memory `self-roundtrip-tests-do-not-prove-interop.md`. - P1 (artifact-identity): v2 verify now derives the protocol id via DataItem::id() (base64url(SHA256(signature))) and requires equality with the requested URL item_id. A node key signs many data items, so a valid signature only proves who signed the response — not that the served item is the one the caller asked to verify. The team memory `verify-against-artifact-id-not-signer.md` is the policy. - P1 (verifier can't read this head's writes): verify_anchor now accepts BOTH the v2 ANS-104 format and the v1 raw-JSON shape the live path on this branch actually writes. The v1 path does a field-equality check on repo, ref_name, old_sha, new_sha, node_did against the persisted row (no signature in v1; Irys storage plus the JSON parse are the integrity guarantee). - P2 (ProbeOutcome in result): AnchorVerifyResult now carries outcome: ProbeOutcome. The HTTP handler maps the structured field to the status string instead of parsing the human- readable error message — any future wording change in arweave_v2.rs can no longer silently reclassify a DefinitivelyAbsent as Indeterminate. - P2 (empty 404 body): classify_404 returns Indeterminate for bodyless 404s. The team memory `distinguish-unknown-from-empty.md` is the policy: a bodyless 404 from a proxy is not a proof of absence, and the recovery policy (permits_reupload -> true on DefinitivelyAbsent) authorizes a paid, irreversible re-upload. - P2 (streaming cap): read_capped_body no longer uses resp.bytes() (which buffers the full response before checking size). It now reads chunk-by-chunk and aborts the moment the cumulative byte count crosses the cap, so a chunked response without Content-Length cannot force unbounded allocation. * fix(node): address three reviewer findings on Twigpine#26 split 2/4 (round 2) * fix(node): address five reviewer findings on Twigpine#26 split 2/4 (round 3) * fix(node): address reviewers round-4 on Twigpine#26 split 2/4 (interop) * fix(node): address coderabbit on Twigpine#26 split 2/4 (raw tags, vint overflow, golden key) * fix(node): enforce artifact-id in probe_item on Twigpine#26 split 2/4 * fix(node): require expected owner for probe Present on Twigpine#26 split 2/4 * fix(node): narrow Twigpine#26 split 2/4 to codec and policy types * fix(node): silence dead-code for deferred slice API on Twigpine#26 split 2/4 * fix(node): doc and bounds-check cleanup in ans104 on Twigpine#26 split 2/4 * docs(node): point tests doc at Ed25519 golden script on Twigpine#26 split 2/4 * fix(node): strict owner lengths, sign-before-hash on Twigpine#26 split 2/4 * fix(node): keep ans104 tags and field widths consistent on hash and encode DataItem keeps both the decoded `tags` and the original Avro `raw_tags`, both public. After a parse, changing `tags` left deep_hash and verify_data_item using the stale raw bytes, and to_binary wrote a header tag count that no longer matched the payload, so the frame failed to parse back. Take the tag block for hashing and encoding from one helper that decodes `raw_tags` and requires it to equal `tags`, and fail closed otherwise. The header count comes from the same validated pairs. Parsed multi-block and size-prefixed items still re-encode byte-exact and keep their signatures. deep_hash now also rejects a non-empty target or anchor that is not 32 bytes, as to_binary already does, so sign_data_item fails instead of signing an item that cannot be encoded. The legacy non-base64url anchor fallback is kept. The module and signature-type docs now say binary parsing accepts sigtypes 1 to 4 while signing and verifying are Ed25519 only on this slice. * build(scripts): declare the golden script's direct npm dependencies ans104_golden_ed25519.mjs imports bs58, @noble/ed25519 and base64url, and ans104_golden.mjs imports base64url, but package.json listed only arbundles, so they resolved only through hoisting. Declare them at the versions already locked, update the lockfile root entry (no resolved version or hash changes), and note the regeneration command in the golden output file. --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com>
…e#537) * docs: state that GITLAWB_ENFORCE_OWNER_PUSH defaults to true PR Twigpine#330 flipped the code default to true and updated .env.example, the README env-var table, the AWS guide, the macOS app, and docs/RUN-A-NODE.md, but two prose mentions in operator-facing docs still describe the opposite. SECURITY.md line 57 ("Repository write authorization defaults" section) and README.md line 69 (Known limitations) both say the flag "defaults to `false`", which contradicts the live runtime and the rest of the same files. Reviewer 2 closed Twigpine#224 with this as the P2 finding to fix in the config-and-policy split. The correct text already exists in the README env-var table; the prose restatements were missed. Bring the prose into line with the code. - SECURITY.md: rewrite the section to lead with the default being true and describe `false` as the explicit rolling-upgrade override. - README.md: rewrite the known-limitations bullet to state the default is true and the `false` opt-out is a rolling-upgrade only. - Add `crates/gitlawb-node/tests/owner_push_default_docs.rs`: a regression guard that reads the four surfaces (clap default, .env.example, README known-limitations prose, README env-var table, SECURITY.md) and asserts each one says the default is `true`. Reverting either doc edit turns one named test red; flipping `default_value_t` back to `false` turns the env example test red. Each surface is checked independently so a stale doc lies alone. No code change. No new migration. No dependency change. Compiles clean, clippy clean under `-D warnings`. Refs: closes the P2 finding from the review of Twigpine#224. * fix(docs): clarify default value consistency for GITLAWB_ENFORCE_OWNER_PUSH * docs(readme): drop the default-open roadmap line and the em-dash punctuation --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com>
…plit 3/4) (Twigpine#548) * fix(node): version ref_certificates for forward compat (Twigpine#26 split 3/4) Reviewer 2 closed PR Twigpine#224 on 2026-08-28 with a directive: split into four narrow PRs. This is Split PR 3 (certificate / CLI compatibility). No findings are assigned to it; it is required because the protocol/client work is independently reviewable and should not remain coupled to post-receive recovery. The wire-format problem: the ref-cert payload is a fixed 7-field JSON blob the node signs. Future changes (additional pusher attestations, content-binding fields, secondary signatures) would either break every existing cert or be impossible to ship without a flag day. Versioning the cert makes the format forward-compat and explicit. NEW MIGRATION v28 ref_certificates gains `version INTEGER NOT NULL DEFAULT 1`. DEFAULT 1 means an existing row reads as v1 with no backfill needed; NOT NULL means a missing value is a hard error rather than a silent v0. NEW FIELD ON RefCertificate `version: u32`. v1 is the pre-versioning 7-field payload (no `version` key in the signed JSON, so a v1 cert is byte-for-byte identical to today's cert). v2+ certs will include a `version` key and any new fields, both backwards-compatible: an old client reading a v2 cert sees the unknown `version` field and either ignores it (graceful) or refuses to verify (explicit). API CHANGES - list_certs and get_cert now include `version` in the JSON response. - issue_ref_certificate sets `version: 1` on every new cert. GL CHANGES - gl cert show reads the `version` field; missing field defaults to 1 (forward-compat with old servers). v1 certs go through the unchanged verify path. v2+ certs are explicitly rejected with an "upgrade the client" message rather than silently guessing the payload shape — a future client that supports v2 cannot accidentally treat a v1 cert as v2. GOLDEN-FORMAT FIXTURE v1_payload_matches_frozen_canonical_form pins the v1 signing payload byte-for-byte. Reverting this to a v2 shape (adding a `version` key) is what the versioned format forbids: an old client reading a v1 cert must not see a `version` key in the signed JSON. The same byte-form is what gl's verify_signature reconstructs in crates/gl/src/cert.rs, so any drift between node and CLI breaks the test instead of silently rendering every real cert INVALID. TESTS - 3 v1_payload_tests in crates/gitlawb-node/src/cert.rs: frozen-canonical-form, round-trip structural, default version is 1. - 2 new tests in crates/gl/src/cert.rs: missing_version_defaults_to_1_and_verifies, explicit_version_1_takes_v1_path. - 12 existing ref_certificate_tests still pass with the version column added. Compiles clean, 1091 tests pass with 0 regressions, gl 4 cert tests pass, clippy clean under -D warnings, fmt clean. Cross-PR overlap (declared in the PR description): - Twigpine#134 (anchors auth): independent. The cert API is unchanged in shape; auth gates apply as before. - Twigpine#285 (advisory-lock session affinity): independent. - Twigpine#306 (Content-Digest on signed requests): independent. - Twigpine#314 (small-order Ed25519): independent. v1 certs verify through the same path, which already enforces the small- order check from Twigpine#314 on the embedded node DID. - Twigpine#324 (libp2p keypair persistence): independent. - Twigpine#325 (gossip ref-update auth): independent. PR 1's signed HTTP push is the durable-intent producer; this PR owns the cert shape. The two PRs read and write the same cert table but do not step on each other (PR 1 calls issue_ref_certificate_idempotent; this PR keeps the deterministic cert id but does not change who calls what). - Twigpine#382 (replication withheld-subtree trees): independent. * fix(node,gl): address review feedback on cert version compat - Drop the duplicate `version` ALTER from the v1 migration bundle; v28 is now the sole owner of the column. - Add a v28 upgrade-path test (v25/v26 pattern) that seeds a legacy pre-v28 ref_certificates row, drops the column + migration record, re-runs migrations, and asserts DEFAULT 1 + legacy row reads as v1. - Extract a shared `v1_signing_payload` builder so the live signer and the frozen-vector test share one literal; a regression in the signer now fails the test. - Tighten gl's `version` parsing: missing key → v1, present value must be a JSON integer that fits in u32 and equals 1. Null, strings, floats, overflow, and unknown versions all return Err instead of collapsing to v1 (Reviewer 2). Add a truth-table test pinning every cell of the parser. - Extend the v1 forward-compat tests to actually round-trip through `verify_signature`, and add a regression test that pins the verdict branch rejects v2 even when the v1 signature would otherwise verify. * fix(cert): address CI failures on PR Twigpine#386 - cargo fmt --all (CI fmt check was red on the prior commit) - v28 upgrade-path test: switch version_column_default from fetch_one to fetch_optional, so a pre-v28 database with the column dropped returns Ok(None) rather than RowNotFound. The precondition assertion is what detects a missing column; RowNotFound would have masked a regression. - clippy `needless_borrow` on the v2 verdict-branch test: drop the leading `&` on `cert_json["signature"].as_str()`. * fix(gl,node): make the v2 rejection and issuer anchor real, and test them as such Round-2 findings, all three demonstrated by execution: - cmd_show's Ok(v != 1) verdict arm was dead code (parse_cert_version admits only 1), and the test aimed at it copied the match instead of calling the command. The match now mirrors the parser's contract and two end-to-end mockito tests drive cmd_show itself: a version-2 cert with a valid v1 signature fails --verify with the version named. - --verify no longer accepts the queried node's self-reported DID as a trust anchor: the node that served a forged cert can serve a matching self-report, and that fallback passed a wholly forged certificate with exit 0. --verify now requires --expect-node before claiming a trusted issuer, with a test proving the refusal (and the anchored positive control passing). - The node-side version test asserted version == 1 on its own literal; it now cross-checks the issuing construction: a version-1 claim must verify against the v1 payload builder's bytes rebuilt from the cert's own fields, and the v1 payload must carry no version key. * fix(node,gl): address round-2 reviewer findings on Twigpine#26 split 3/4 cert compat BREAKING CHANGE: gl cert show <id> --verify now requires --expect-node <did>; the node's self-reported DID is no longer trusted as a cert anchor. * fix(node): bind cert v2 test to live issuer via sqlx::test * fix(node,gl): address round-3 reviewer findings on Twigpine#26 split 3 cert compat - Stamp new certs v1 until v2 verifier ships; v2 issuance refused by shipped gl - Add v2_signing_payload with version in signed bytes, downgrade test - Verify signature in well-formed test, add DB read-back to issuer test - Fix README/db comments and version-refusal message * fix(gl): route cert list through error_for_status; correct cert show example Round 4 reviewer findings on Twigpine#26 split 3 cert compat: - P2 (cert.rs:113): cmd_list called .json() straight through on the cert list endpoint and let a gated 404 (whose body lacks a `certificates` key) fall into the empty-list branch, printing "No ref certificates" with exit 0. Wire cmd_list through the same error_for_status() gate that cmd_show and resolve_cert_id already use, and pin the new behavior with a load-bearing test (`cmd_list_surfaces_gated_404_as_err`) that drives cmd_list end to end against a 404 mock. cmd_show is unchanged. - P3 (README.md:314): the documented `gl cert show <id> --verify` example bound the certificate ID as the REPO positional, because the parser is `gl cert show [OPTIONS] <REPO> <ID>`. Insert the missing `<repo>` positional so the example matches the CLI parser. The original BREAKING CHANGE footer in bb553b6 carried the same shortened `gl cert show <id> --verify` example; this commit's footer supersedes it with the canonical command, which is what release-note scrapers read. The round-2 commit message itself is left in history unchanged; the new footer is the one callers should rely on. BREAKING CHANGE: gl cert show <repo> <id> --verify requires --expect-node <did>; the queried node's self-reported DID is no longer accepted as a cert anchor. * style(gl): fix fmt in cert list response parsing * fix(gl): label version refusals distinctly and clean cert show help A cert declaring an unsupported version printed INVALID, which reads as a failed Ed25519 check even though verify_signature never ran. The display now prints UNSUPPORTED VERSION for version refusals and keeps INVALID for real signature failures. The Show help no longer cites review rounds or commit history. * docs(node): align RefCertificate.version rustdoc with the signed-bytes v2 contract --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com> Co-authored-by: Kevin Codex <kevin@gitlawb.com>
…split 1/4) (Twigpine#558) * fix(node): durable post-receive outbox at the DB layer (Twigpine#26 split 1/4) Reviewer 2 closed PR Twigpine#224 on 2026-08-28 with a directive: split the work into four narrow PRs. This is Split PR 1 (durable post-receive lifecycle) at the DB layer; the handler refactor in crates/gitlawb-node/src/api/repos.rs:2007 (git_receive_pack) lands in the next slice so the test can drive the failure injection end-to-end. The pre-outbox crash window the reviewer flagged: receive_pack can apply a ref to disk and return Ok, and a process exit, a dropped future, or a DB failure before the bookkeeping at crates/gitlawb-node/src/api/repos.rs:2361 (push event + cert + webhook) loses the recovery record. Startup drain enumerates only sources written from that bookkeeping, so it cannot reconstruct the missing work. The partial fallback that re-derives from a row present in the bookkeeping substitutes did:key:recovered and an empty attestation, which is not equivalent to the original authenticated push. This commit adds the durable boundary the handler will lean on. NEW TABLE pending_ref_transitions (migration v27): - Written by the handler BEFORE smart_http::receive_pack, in state 'prepared', carrying the verified pusher DID, the raw RFC 9421 signature header, signature-input, and content-digest that authorized the push, the request id, and the parsed ref update. - The handler transitions the row to 'applied' on receive_pack Ok or 'cancelled' on Err. The drain reads only 'applied'. - A failed or cancelled receive-pack therefore leaves the row in 'prepared' or 'cancelled', which the drain never promotes. This is what closes the reviewer's second proof ("a failed or cancelled receive-pack does not turn a prepared intent into completed accounting or anchoring"). NEW TABLE anchor_jobs (migration v27, owned by PR 1, consumed by PR 2): - One row per (repo_id, ref_name, old_sha, new_sha) transition. PR 1 inserts it on 'applied'; PR 2 reads it and updates claimed_at. - ON CONFLICT (id) DO NOTHING makes the insert idempotent on the deterministic id, so a recovery re-pass cannot create a second upload request. This is the handoff boundary; the bundler call itself is PR 2. NEW DB METHODS on Db: - insert_pending_ref_transitions: writes one 'prepared' row per ref update, returns the persisted rows. - mark_pending_ref_transitions_applied / _cancelled: state flip, gated on the FROM state, idempotent. - list_pending_ref_transitions_applied: drain query, oldest first. - delete_pending_ref_transition: called by recovery after the artifacts land; a third pass is a no-op. - record_push_with_id: ON CONFLICT (id) DO NOTHING on the deterministic id. - insert_ref_certificate_idempotent: ON CONFLICT (repo_id, ref_name) DO NOTHING, returns None if a live-path cert already exists. - insert_anchor_job_idempotent: ON CONFLICT (id) DO NOTHING on the deterministic per-transition id. NEW HELPERS in db/mod.rs: - deterministic_id: SHA-256 hex with an ASCII Unit Separator between fields so two distinct tuples never collide on prefix overlap. - push_event_id_for, ref_cert_id_for, anchor_job_id_for: the derived ids above, one helper per artifact so a caller cannot derive a wrong id by mistake. NEW STRUCTS: - PendingRefTransition: the row shape. - AnchorJob: the handoff row shape. - pending_state: const strings ('prepared' / 'applied' / 'cancelled') shared by tests, the producer, and the drain so a typo on one side cannot silently mismatch the other. NEW TESTS in db::pending_ref_transition_tests (8 tests, all green): - insert_then_mark_applied_flips_state_for_every_ref: producer contract. - mark_applied_is_idempotent_on_repeat: re-fire is a no-op. - cancelled_rows_are_not_returned_by_the_drain: reviewer's second proof at the DB layer. - prepared_rows_are_not_returned_by_the_drain: same proof for the pre-flip state (handler crashed before reaching post-Ok). - mark_cancelled_is_idempotent_on_repeat: counterpart. - drain_then_re_derive_is_idempotent: reviewer's first proof at the DB layer. Inserts a row in 'applied' state directly via insert_pending_ref_transition_for_test, drains it, derives the artifact ids twice, exercises record_push_with_id and insert_anchor_job_idempotent directly, asserts exactly one push event row and exactly one anchor job row regardless of how many times the drain runs. - deterministic_id_avoids_prefix_overlap_collisions: the separator regression test. - push_event_id_for_is_stable: derived ids match across calls and differ on each varied input. OTHER: - Make RefUpdate and its fields pub(crate) so the DB methods can iterate the parsed ref updates. No public API change. NOT IN THIS SLICE (the handler refactor, next commit): - The receive-pack handler does not yet call insert_pending_ref_ transitions before the receive_pack call, nor mark_applied / mark_cancelled after. The DB layer is in place for it; the handler will call these methods and the startup drain will be wired in main.rs. - The startup drain in main.rs is not yet called; it will iterate list_pending_ref_transitions_applied, re-derive the artifacts, and delete the row. - The cert/push event issuance in cert.rs and the bookkeeping in api/repos.rs:2361 are not yet changed to use the deterministic ids. The helper functions exist and are tested; the callers follow. Compiles clean, clippy clean under -D warnings, fmt clean. * fix(node): wire durable outbox into the receive-pack handler (Twigpine#26 split 1/4) This is the handler-level half of Split PR 1. The previous commit added the migration and the DB methods; this one threads them through crates/gitlawb-node/src/api/repos.rs:2007 (git_receive_pack), the cert issuer, and the startup drain. CHANGES IN THE HANDLER ====================== In git_receive_pack, AT THE LAST POSSIBLE MOMENT before the smart_http::receive_pack call, the handler now: 1. Generates a per-handler request_id (UUID). 2. Captures the raw Signature, Signature-Input, and Content-Digest headers from the request. 3. Calls db.insert_pending_ref_transitions(request_id, ...) which writes one row per ref update in state 'prepared'. The receive_pack call runs as before. After it returns: 4. On Ok: db.mark_pending_ref_transitions_applied(request_id) — the row is the ONLY thing that promotes a 'prepared' row to 'applied', and the drain reads only 'applied' rows. A process crash before this call leaves the row in 'prepared', which the drain never promotes. 5. On Err: db.mark_pending_ref_transitions_cancelled(request_id) — a failed receive_pack leaves the row in 'cancelled', which the drain never promotes. This is what closes the reviewer's two proofs: Proof 1 (crash window): if the process dies after mark_pending_ref_transitions_applied but before the bookkeeping writes, the row is in 'applied' and the next startup drain re-derives the push event, the per-ref certificate (carrying the ORIGINAL pusher DID, not a placeholder), and the anchor handoff. The drain uses the persisted authentic pusher DID and signature header, not a recovered placeholder. Proof 2 (failed receive-pack): the row is only ever flipped to 'applied' in the explicit Ok branch above. A 'prepared' or 'cancelled' row is invisible to the drain, so a failed or dropped receive_pack cannot turn a prepared intent into completed accounting or anchoring. BOOKKEEPING IS NOW DETERMINISTIC-ID =================================== The post-Ok bookkeeping at api/repos.rs:2448 now uses: - record_push_with_id with push_event_id_for(request_id, first_ref) — ON CONFLICT (id) DO NOTHING, so a recovery re-pass is a no-op. - issue_ref_certificate_idempotent with ref_cert_id_for(request_id, ref_name) — ON CONFLICT (repo_id, ref_name) DO NOTHING, returns None if a live-path cert already exists. - insert_anchor_job_idempotent with anchor_job_id_for(repo_id, ref_name, old_sha, new_sha) — the per-transition tuple key, so two pushes to the same ref produce one anchor upload per landed state. The legacy entry points (record_push, issue_ref_certificate, insert_ref_certificate) remain for callers that prefer a fresh UUID per cert; they are #[allow(dead_code)] for the PR 3 cert/CLI compat pass to decide whether to keep or remove. STARTUP DRAIN ============= crates/gitlawb-node/src/main.rs calls durable_outbox::drain_pending_ref_transitions(state, 1000) ONCE before serving, after migrations and after the existing peer / quarantine prunes. Non-fatal: a transient drain failure logs and leaves the rows for the next startup. durable_outbox::drain_pending_ref_transitions reads every 'applied' row, calls derive_one (which re-derives the three artifacts using the persisted authentic pusher DID and signature header), then deletes the row. A second drain pass is a no-op for both the artifacts (idempotent inserts) and the row (gone after the first pass). NEW END-TO-END TESTS ==================== crates/gitlawb-node/src/durable_outbox.rs adds three end-to-end tests in drain_tests, complementing the eight DB-layer tests in db::pending_ref_transition_tests: - drain_re_derives_all_three_artifacts_for_an_applied_row: the reviewer's first proof. Inserts a row in 'applied' state (the crash window), drains, asserts exactly one push event row, exactly one cert row carrying the original pusher DID (not a placeholder), and exactly one anchor job row. Asserts the deterministic cert id matches. Asserts a second drain pass is a no-op. - cancelled_row_produces_no_artifacts: the reviewer's second proof for the cancelled state. A row in 'cancelled' (receive_pack returned Err) is invisible to the drain. - prepared_row_produces_no_artifacts: the reviewer's second proof for the prepared state. A row in 'prepared' (handler crashed between insert_prepared and the post-Ok branch) is invisible to the drain. Each test names the invariant it pins and the production line it covers. Reverting that line turns the named assertion red. Compiles clean, 1099 tests pass with 0 regressions, clippy clean under -D warnings, fmt clean. Cross-PR overlap (declared in the PR description): - Twigpine#134 (anchors auth): composes. The /arweave/anchors route already requires auth; this PR does not change the route. - Twigpine#285 (advisory-lock session affinity): composes. The durable intent is written inside the same handler that holds the lock from Twigpine#285; no changes to the lock layer. - Twigpine#306 (Content-Digest on signed requests): composes. PR 1 persists the Content-Digest header that Twigpine#306 makes mandatory. - Twigpine#314 (small-order Ed25519): independent. PR 1's tests use strong keys. - Twigpine#324 (libp2p keypair persistence): independent. PR 1 does not touch p2p identity. - Twigpine#325 (gossip ref-update auth): independent. PR 1's signed envelope is the HTTP-side equivalent, not the gossip-side. - Twigpine#382 (replication withheld-subtree trees): independent. PR 1 does not touch replication or pin selection. * fix(node): address four reviewer findings on Twigpine#26 split 1/4 - P1-A: add startup reconcile step that promotes `prepared` rows to `applied` when the on-disk ref matches the row's `new_sha`. The recovery drain (which only reads `applied` rows) can now pick up a ref that landed when the live handler's `mark_pending_ref_transitions_applied` call errored or was interrupted. Strict SHA equality is the load-bearing check — a `prepared` row whose target did NOT actually land stays `prepared`. - P1-B: route the live handler's cert issuance through `cert::issue_ref_certificate` (the upsert) instead of `issue_ref_certificate_idempotent` (DO NOTHING). A re-push to the same ref now updates the cert's `old_sha` / `new_sha` / `pusher_did` / `issued_at` / `signature` to the new transition while preserving the deterministic `cert_id`. The recovery drain keeps the idempotent variant; both paths collapse to one row. - P2-A: refactor the drain into a `drain_pending_ref_transitions_with` testable seam that does per-row log-and-continue, and add `drain_pending_ref_transitions_all` that loops `DRAIN_PER_PASS_LIMIT=1000` rows for `DRAIN_MAX_PASSES=10` passes. A failing row no longer stalls the batch; a backlog above 1000 rows is fully processed across passes. - P2-B: add a `first_ref_name` column to `pending_ref_transitions` via migration v28. The live handler hoists a `first_ref_name` local and persists it on every row of the same `request_id`. The drain's `derive_one` keys the push event id on `row.first_ref_name` instead of `row.ref_name`, so live and recovery produce the same id and `ON CONFLICT (id) DO NOTHING` collapses a multi-ref push to one push event row (and one trust- score bump). Cert and anchor ids stay per-ref / per-transition. * fix(node): address four reviewer findings on Twigpine#26 split 1/4 (round 2) * fix(node): address reviewer round-3 findings on Twigpine#26 split 1/4 - P1: Delete outbox rows after live durable effects complete so they don't replay on every restart - P1: Parse git report-status for per-ref ok/ng results; mark only proven rejections as cancelled, uncertain outcomes as recoverable - P1: Introduce 'uncertain' state for receive-pack errors where some refs may have landed; reconcile checks these against disk at startup - P2: Promote deletion transitions during reconcile (new_sha == ZERO_SHA with absent ref = successful deletion) - P2: Loop reconcile across multiple passes so backlogs beyond the first page are processed in the same startup Closes review round 3 findings from reviewer-1 and reviewer-2. * fix(node): fix CI failures from round-3 changes - Add COMMENT ON TABLE to v29 migration so migration_bodies_are_non_empty passes - Return error on non-zero receive-pack exit (preserving backward compat with tests that expect Err(AppError::Git(_))) while still parsing report-status for outbox row handling * fix(node): update inv22 gate tests for receive_pack_raw refactor - F3 gate: scan for smart_http::receive_pack_raw( instead of receive_pack( - U5 gate: scan for all_refs_ok instead of receive_result.is_ok() * fix(node): require reflog proof before the reconcile promotes a row (Twigpine#26 split 1/4) Closes the second half of the reviewer's round-3 [P1]: the startup reconcile still promoted on `disk_refs.get(ref) == row.new_sha`, which also matches a coincidental current tip (old=B, new=A while the ref is already A). That is not an exotic case — it is the ordinary shape of a REJECTED push, since git refuses an update whose expected old value does not match the ref. Both the SHA check and the MAX_RECONCILE_AGE window pass, the push never happened, and the drain then writes a push event, a signed certificate and an anchor for it. The live path proves landings from git's report-status body; that body is gone by the time the reconcile runs, so the proof is re-derived from the repository. `git::store::ref_reflog_entries` reads `logs/<ref>` and a row is promoted only when an entry carries its exact `old -> new` pair, stamped at or after the row was written (REFLOG_CLOCK_SKEW absorbs git's whole-second truncation against the row's sub-second `created_at`). The timestamp floor is what separates a landing from a later push that re-introduced the same pair: proof must postdate the intent it proves. Deletions stay exempt from the reflog half and keep the absence-plus-age rule already in place — git removes a ref's reflog along with the ref, so no other evidence can exist for one. `init_bare` now sets `core.logAllRefUpdates`, which bare repos default off; a repo with no reflog yields no proof and its rows stay put for human-attended recovery rather than becoming accounting the node cannot substantiate. Tests: reconcile_refuses_a_coincidental_tip_with_no_reflog_proof, reconcile_promotes_a_row_the_reflog_proves, reconcile_leaves_a_row_prepared_when_the_repo_keeps_no_reflog, reconcile_refuses_a_reflog_entry_older_than_the_row, reconcile_still_promotes_a_landed_deletion_which_can_have_no_reflog, init_bare_keeps_reflogs_so_a_landing_can_be_proven, ref_reflog_entries_is_none_when_the_repo_kept_no_log. No migrations added or renumbered. * fix(node): walk the reconcile backlog on a keyset cursor (Twigpine#26 split 1/4) The multi-pass reconcile looped but never advanced. Each pass re-issued the same cursor-less `LIMIT n` query, and unlike the drain — which deletes every row it finishes, so its next page is always new work — the reconcile leaves every row it cannot promote exactly where it was. One unprovable row at the head of the ordering therefore pinned page one on every pass, and the backlog behind it was never examined at all, which is the condition the multi-pass loop was added to remove. Rows keep ageing toward MAX_RECONCILE_AGE while they wait, so "picked up next restart" can mean "never recovered". Pages now resume strictly after the `(created_at, id)` of the last row EXAMINED, via `list_pending_ref_transitions_prepared_or_uncertain_after`. A keyset cursor rather than an OFFSET because rows leave the set underneath the walk when they are promoted: an absent row simply does not appear on a later page, and it can never shift an unvisited row into a page that was already read. Pass budget, per-pass limit, short-page exit and the residual-backlog warning are unchanged. This matters more with the reflog gate in place: a landing in a repo that keeps no reflog is PERMANENTLY unprovable, so before this it would jam page one on every startup rather than once. Tests: reconcile_all_walks_the_backlog_past_the_first_page, reconcile_all_advances_past_an_unprovable_row (whose blocker is a no-reflog row, the class the gate introduces). * fix(node): bound the reflog read to a recent tail (Twigpine#26 split 1/4) `ref_reflog_entries` slurped the whole reflog. A reflog grows one line (~150 bytes) per ref update and how often a ref is updated is PUSHER-controlled, so that was an attacker-sized allocation, taken once per stranded row, at startup — the moment the node can least absorb it. Every other attacker-influenced read in this crate is bounded (read_body_capped, MAX_SIGNATURE_ENTRIES, the pin/walk budgets); this one should be too. The bound costs nothing in proving power, which is why it is a ceiling rather than a tradeoff: the landing gate only ever accepts an entry stamped at or after `created_at - REFLOG_CLOCK_SKEW`, and reflogs are append-ordered oldest-first, so every entry that could possibly qualify is at the tail. The read now seeks to the last REFLOG_TAIL_BYTES (256KB, ~2000 recent updates to one ref) and `take`s that many bytes, so the allocation stays bounded even if the file grows between the stat and the read. A window into the middle of a file starts mid-record, so the bytes before its first newline are discarded: without that, a boundary landing inside a record's old SHA leaves a suffix that still tokenizes and yields a truncated SHA as a bogus `old -> new` pair — and landing proof is an exact pair match, so a bogus pair is a fabricated proof. An entry older than the window reads as absent, which is the safe direction: the caller treats "no matching entry" as unproven and leaves the row where it is. `Ok(None)` for a missing file, and the absence-of-evidence contract, are unchanged. Tests: ref_reflog_entries_reads_whole_records_from_the_recent_tail (lays the file out so the boundary lands 20 bytes into a record's old SHA — the alignment that would otherwise parse), an_entry_older_than_the_reflog_tail_reads_as_unproven. * fix(node): read the report through both side-band frames The report-status is framed twice whenever the client negotiates side-band-64k, which `git push` over smart HTTP does: the outer side-band envelope carries a stream that is itself pkt-line encoded. One strip pass left the first line as `000eunpack ok`, so the unpack check failed and parse_report_status returned None — read by the caller as "no report", which keeps every declared ref. The per-ref landing gate was therefore inert for exactly the pushes it exists to filter. The regression test is a byte-for-byte capture from git 2.50.1 rejecting one ref of a two-ref push, because a hand-written single-framed fixture parses under the old code and hides the case. * fix(node): address round-4 reviewer findings on Twigpine#26 split 1/4 * fix(node): rustfmt round-4 reviewer fixes * fix(node): remove empty line after doc comment * fix(node): drop redundant reflog gate; implicit-ok on exit-zero no-report * fix(node): address round-5 reviewer findings on Twigpine#26 split 1/4 * fix(db): v30 migration — add receive_pack_requests table and ordinal column The reviewer's round-5 finding: the push event identity was encoded into a mutable per-ref column (first_ref_name) whose correct value is knowable only after git, and the correction was not committed atomically with the per-ref outcomes. A crash between git updating a later ref and the rewrite left the durable rows naming the rejected ref, and derive_one's row.ref_name == row.first_ref_name guard then meant no push event ever landed for the accepted child. Per the state-transition model at .gravirei/plans/state-model-durable-post-receive.md, this migration introduces the request-level record that owns the push event and the trust score, and extends the per-ref child with an ordinal column the drain and the effect executor walk together. first_ref_name is dropped; the push event id is keyed on (request_id, accepted_ordinal) and lives on the request row. The next commit (handler rewrite) wires the live path and the recovery drain against the new tables. BREAKING CHANGE: pending_ref_transitions.first_ref_name is dropped. Callers that read or write that column must move to receive_pack_requests.accepted_ordinal. * fix(node): rewrite receive-pack handler against the request-level model (Twigpine#26 split 1/4 step 2) The v30 migration added receive_pack_requests and dropped first_ref_name, but no Rust code used either. The handler still ran a four-branch per-ref state flip plus a first_ref_name rewrite after git returned, which is the mixed-outcome bug a push with the first ref rejected exposes: the live path and the drain computed different push-event ids. - Insert a receive_pack_requests row in state `received` BEFORE git runs (insert_receive_pack_request), carrying the request bytes and the SHA-256 of the body. Crash between intent and git return is now recoverable via the reconcile step. - Drop the first_ref_name rewrite at api/repos.rs:2454. The push event identity is now (request_id, accepted_ordinal); accepted_ordinal is computed once at the per-ref state flip. - Re-key push_event_id_for and ref_cert_id_for on (request_id, ordinal). Anchor job id stays per-transition. - Transition the request row to outcomes_committed (with parsed_report and accepted_ordinal) or rejected_at_git (no report + non-zero exit) in the same handler tail as the per-ref state flip. The drain (step 3) picks up outcomes_committed rows; today the live path also runs the per-ref effects inline. - Stub the step-3 surface (mark_request_effects_pending, complete, list_receive_pack_requests_due, update_request_attempt) with #[allow(dead_code)] + contract-pin comments so the rewrite is bisectable: reverting this commit restores the pre-rewrite live path. The U5 gate (replication tail spawn inside push_succeeded, before guard.release) is preserved; inv22_replication_tail_spawns_at_the_ durability_boundary stays green. * fix(node): use raw bytes for request_bytes_hash (Twigpine#26 split 1/4 step 2) The v30 migration defined request_bytes_hash as BYTEA but the handler was binding a hex-encoded String. Postgres rejected the insert with "column request_bytes_hash is of type bytea but expression is of type text" and the handler shed the push with a 503. Switch the handler to bind the raw 32-byte digest and update the ReceivePackRequest struct field type to Vec<u8> to match. The drain-side test fixture already used Vec<u8> and needs no change. Pinned by the bin test suite going from 24 failures (all the receive-pack-cap tests that send a 4-byte body) to 0 regressions on the new model. * fix(node): factor out apply_request_effects; drain walks receive_pack_requests (Twigpine#26 split 1/4 step 3) The step-2 commit moved the request row to be the unit of work but left the per-ref effects fan-out inline in the handler and the drain walking pending_ref_transitions per-ref. Step 3 factors the effects fan-out into apply_request_effects(state, request_id) and rewrites the drain to walk receive_pack_requests. - apply_request_effects lives in durable_outbox.rs; the live handler in api/repos.rs and the drain both call it. Idempotent on (request_id, accepted_ordinal). Returns EffectsOutcome::{Done, Nothing, Retry}. - Drain switches from list_pending_ref_transitions_applied + derive_one to list_receive_pack_requests_due + apply_request_effects. derive_one and the per-ref drain entry points are deleted. - The step-3 stubs in db/mod.rs (mark_request_effects_pending, mark_request_complete, list_receive_pack_requests_due) lose their #[allow(dead_code)] annotations; every stub has a caller in this PR. - Reconcile (reconcile_prepared_from_disk_all) is unchanged. The crash window between intent durable and outcomes commit still routes through per-ref reflog proof. - Drain tests rewritten to stage receive_pack_requests rows; the per-ref fixtures (make_row, lookup_accepted_ordinal) are gone. - New inv26_step3_live_and_drain_share_apply_request_effects assertion pins the live/drain sharing and the per-request drain walk. * fix(node): bounded retirement purge for terminal request rows (Twigpine#26 split 1/4 step 4) The v30 partial index idx_receive_pack_requests_completed_at exists but no code reads it. Step 4 wires a periodic purge task that deletes terminal `complete` and `rejected_at_git` rows older than the retention window, along with their per-ref children. `quarantined` (a step-5 state) is never purged by the timer. - `purge_completed_receive_pack_requests(older_than, limit)` — deletes parent requests using the v30 partial index. - `purge_completed_pending_ref_transitions(older_than, limit)` — deletes children of purged parents, gated on the child's own applied_at / cancelled_at. - `purge_request_queue(db, retention_days, limit)` — orchestrator in durable_outbox.rs that calls both and returns the totals. The deletion order (parents first, then children) is the contract; a crash mid-purge leaves orphaned children that the next pass will pick up. - `spawn_queue_lifecycle_sweep` in main.rs — periodic task on the same detached pattern as `spawn_legacy_cid_sweep`, 24-hour interval, shutdown-aware. The interval matches the spec's "one per cluster per day" target. - Config knobs `queue_retention_days` (default 7, range 1..=365) and `queue_purge_batch` (default 1000, matches DRAIN_PER_PASS_LIMIT). - 3 new `durable_outbox::drain_tests::purge_*` tests pin the contract: only old terminal rows are deleted, the second pass is a no-op, and a row inside the retention window survives. - New `inv26_step4_queue_lifecycle_purge_is_wired` gate asserts the wiring (main.rs calls the purge, the DB helpers exist, the Config knobs exist). * fix(node): reference-transaction marker + quarantined state + failure matrix (Twigpine#26 split 1/4 step 5) Steps 2-4 gave the request row the unit of work, the shared executor, and the bounded retirement policy. Step 5 closes the evidence gap: the reconcile now requires a per-request marker ref (refs/gitlawb/requests/<id>) whose value matches request_bytes_hash. A missing or mismatched marker quarantines the request; an operator reclassifies it. - v31 migration: adds `quarantined` to the state vocabulary and a partial index for operator queries. - Handler writes the marker ref before git-receive-pack; the marker is causally bound by being in the same async task as the receive-pack call. The marker's value is content-addressed (git hash-object of the request bytes), so the gate compares consistent SHAs on both sides. - git::store::read_ref reads a single ref's value, returning Ok(None) for absent refs. Used by the marker gate. - git::store::marker_value_for computes the content-addressed marker value; both the live handler and the reconcile use it so the write and the read agree. - Reconcile gains a marker gate between the age check and the reflog proof. Mismatch or absent ⇒ mark_request_quarantined + mark_children_rejected_for_quarantined_parent. - effects_max_attempts bound (config knob, default 8) flips retry-stuck requests to `quarantined` after N attempts, closing the infinite-retry DoS window. - New failure_matrix_tests submodule covers the spec's outcome × ref-kind × exit-point × recovery-scenario matrix (6 cells). - New inv26_step5_marker_quarantine_and_bound_are_wired gate asserts the marker gate, the bound check, the handler's pre-receive-pack ordering, and every load-bearing helper. - Existing 7 reconcile tests updated to stage a marker ref via the new `stage_marker` test helper. * fix(node): converge durable post-receive to singular request-level lifecycle Unify producer, evidence, aggregate outcome, executor, retry, and retirement on the request row: atomic intent and outcome commits, synthetic normalized reports for implicit-ok/reconciled paths, reconcile parent promotion with fail-closed deletions and competing-claimant guard, retry progression from both executable states with backoff/quarantine, terminal-parent-gated purge with marker cleanup and hidden refs, minimal digest-only intent plus request-level RFC9421 proof (v32). * fix(node): harden request occurrence lifecycle, proof, retry, and retirement Reject incomplete report framing and omitted refs as indeterminate, preserve uncertain children on completion, terminalize all-rejected, bind reconcile with landing history, durable proof with ack gate, occurrence-keyed anchors, background due loop, deletion quarantine with operator resolve, children-before-parent purge with marker tombstones, recovery prereq upgrade check, and bounded marker runner. * fix(node): keep recovery prereq upgrade best-effort on push path Refusing pushes when git config upgrade fails broke fake-git and non-repo disk-path tests with 503. Reconcile already fails closed on missing reflogs, so downgrade to warn-and-proceed. * fix(node): close reviewer round on outcome authority and lifecycle ownership Gate effects on unpack_ok with cleared ordinal and terminal no-effects, intersect executor with applied children, treat absent report-status as indeterminate regardless of exit, require landing history before child delete, claim due requests with recoverable leases, dedupe webhooks by occurrence ledger, ack-gate proof retention with terminal disposition, fair marker tombstones with dead-letter, kill_on_drop marker bounds, and loud migration version collision guard. * fix(node): load-bearing coverage for intent, guards, and delivery ledger Handler intent test plus inv22 gate so disabling the durable insert turns red; unpack-false, mixed ok/ng, divergent cancelled, partial sibling, and proof-ack tests pin each executor guard; webhook deliveries record sent_at with stale-pending reclaim; stale parsed_report comment corrected. * fix(node): centralize live retry, bound outcome commit, close retention orphans Route live Retry/Err through schedule_request_retry_or_quarantine so attempt progression matches the drain; retry post-git outcome commits 3x before restart repair; block terminal purge while non-terminal children exist; add v27 upgrade-path test. * fix(node): document attended-restart commit policy and pin executor retry arms Track outcome-commit success, skip metrics/touch on failure, correct the lease-expiry comment to startup reconcile; add restart-repair test for stuck received parents plus RED tests for landing-history and proof-ack failure paths. * fix(node): correct marker prereq comment to warn-and-proceed * fix(node): single partial-completion model with disposition gate Split apply_request_effects into phase-1 bundle (push, cert, anchor, history, webhook, proof ack before child deletion) and phase-2 gate that retries while non-cancelled siblings remain; Nothing only when nothing owed. Gate tail, Tigris, and effects on PostGitDisposition with documented replication carve-out; add partial-sibling webhook and disposition tests; fix stale comments. * fix(node): close review round on outbox ordering, marker visibility, and repair reachability Address the P1/P2 review findings: await the webhook delivery claim before accepted-child deletion; exempt the marker namespace from the fail-closed ref peeling guard so visibility walks tolerate blob-pointing request markers; keep request marker deletion bounded in the purge path; promote cross-page siblings via idempotent report superset; run stuck-parent repair when the prepared queue is empty; bound the durable intent write while admission permits are held; remove dead proof verification scaffolding. * fix(node): close review round on pre-git refusal lifecycle, namespace gate, and promotion merge * fix(node): close review round on namespace gate, intent lifecycle, and promotion atomicity Namespace gate hardening: - Refuse non-UTF-8 ref-update lines (git accepts bytes ≥0x80; gate never sees them) - Catch bare 'refs/gitlawb' name (starts_with missed it) - Add deny test for both holes plus refusal wiring verification Intent lifecycle guards: - Add IntentDropGuard: terminalize stranded intents on dropped futures - Bound refuse_receive_pack_before_git to 10s (no statement_timeout on pool) - Guard mirror fetch: hideRefs blocks non-exempt refs/gitlawb/* from +refs/*:refs/* Promotion merge atomicity: - Take row under SELECT FOR UPDATE so merge is atomic with state read - Prevents racing due worker from stranding newly applied children outside report * fix(test): correct inv22 namespace gate assertions to match actual implementation The test was looking for the wrong patterns: - UTF-8 check: error message string, not pre-loop condition - Namespace check: `*r == "refs/gitlawb"`, not `ref_name ==` - Visibility exemption: `&&` logic (both false to exclude), not `||` * chore: cargo fmt * fix(node): phase-aware intent guard, mirror import prune, terminal-state merge * fix(node): guard cleanup retry with fates, fail-closed mirror prune, refusal coverage * fix(node): bound guard attempts, sibling-wait outcome, fail-closed prune enumeration * fix(node): sibling-wait outcome, no-report path, bounded prune and guard retry * fix(node): no-report durability, cert landing order, bounded reconcile git, flush checks * fix(node): gate trust-score bump on a new push event row * chore: update rustls 0.23.37 → 0.23.45 to resolve RUSTSEC-2026-0285 * fix(node): surface git's report-status when a stdin EPIPE follows child exit * fix(node): cap receive-pack ref-update count before per-ref work and intent * fix(node): cap ref-update parsing in-loop and dedupe identical commands * docs(node): correct round-4 comment claims on client outcome and gate cost * fix(node): single replication tail, proven marker taxonomy, claimed webhook delivery (Twigpine#558) - One post_receive_replication_tail per push: the carve-out spawn and the durability-bound spawn are mutually exclusive positions on the same proven-landed ref set; inv22 now fails on any unfiltered or ungated spawn. - Marker reconcile: bounded-read errors (git Err/deadline) stay prepared and retry instead of quarantining as 'missing marker ref'; only a successfully read marker proves missing or mismatch. Err-injection test pins no-quarantine on unproven reads. - Claimed webhook delivery: the synchronous claim phase returns the claimed Webhook rows so the send task never re-lists; a list failure after claim can no longer drop claimed deliveries. Source gate pins it. * docs(node): name claimed-hooks variant in fire_event_occurrence doc * fix(node): order pending_ref_transitions create before outbox alters, fill v29 (Twigpine#558) * fix(node): proven-missing marker taxonomy, v38 upgrade test, report-ok tail fixtures (Twigpine#558) * fix(node): read marker refs via rev-parse so present refs return their oid (Twigpine#558) --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com> Co-authored-by: Kevin Codex <kevin@gitlawb.com>
Summary
Anchor ref updates and encrypted-blob manifests to Arweave as signed ANS-104 data items, and add
/api/v1/arweave/verify/{tx_id}which fetches the anchor from the gateway and validates the embedded certificate chain (node Ed25519 signature, SHA-256prevlinkage, pusher RFC 9421 proof). Also harden the Arweave surface: spec-correct ANS-104 preimages, tamper rejection, fail-closed corroboration, credential redaction on every public output, structural URL handling that rejects fragments, an explicit verified bundler-funding configuration, and durable post-receive jobs persisted before the push is acknowledged.Durability scope is deliberate and narrow: push accounting, per-ref certificates, and the Arweave anchor are the durable unit — a crash between the pack landing and that bookkeeping is recovered by the startup drain. The replication tail (Pinata/IPFS pinning, gossip, GraphQL push, peer notify) remains best-effort and is not part of the durable unit.
Closes #26.
Kind of change
What changed
gitlawb-node
item.rawTagsin the publishedarbundlesgetSignatureData), not a nested[[name, value], ...]list — the nested form is what Arweave layer-one transactions use, and a data item's signature would not reproduce with it. Zero tags is an empty blob, notdeepHash([]). The empty-tags reference vector and a 3-tag interop fixture are produced by the independentarbundlespackage (createData+sign) and pinned as hex in tests; the node's own signer produces items this verifier accepts.{bundler}/tx/{token}as raw signed data items with metadata embedded as tags, paying via thex-irys-paid-byheader (IrysUploadHeaders.PAID_BY). The upload client classifies every outcome at the boundary (Acceptedonly for a well-formed 43-char base64url id; empty/missing/malformed ids and connection drops areUncertain— never success, never a fabricated anchor), and exposes the content-derived ANS-104 data-item id plus a gateway presence probe (anchor_item_present) that the durable job uses to reconcile a possibly-landed upload without paying twice.verify_anchor()validates the full chain against the local DB; refusals are fail-closed — an uncorroborated outer repo/owner identity makes the result invalid, and the raw DB error never reaches the caller. Every send-error and non-success-body path on the upload side is masked throughremote_send_error/remote_response_errorso bundler account/token never leak; the same masking covers verify_anchor's connection and mid-stream errors. Tamper tests flip a signature byte on both the 13-field and 7-field verify paths and assert the specific signature error.record_push, trust score, per-refissue_ref_certificate) and the Arweave anchor now run inside a durable post-receive job.git_receive_packpersists the job row (id, pusher DID, repo, ref updates, RFC 9421 attestation) BEFORE acknowledging the push, then spawnsprocess_post_receive_job; a crash between the pack landing and the bookkeeping is recovered by the startup drain (drain_post_receive_jobsin main), which resets stale rows topendingand replays them. Jobs are claimed atomically (UPDATE ... WHERE status IN ('pending','failed')): two concurrent drainers converge on a single executor per job. Every effect is idempotent:push_eventsis keyed on the job id (ON CONFLICT (id) DO NOTHING), certificate ids are deterministic per (job, ref) withinsert_ref_certificate_txidempotent, and a certificate issuance failure now fails the job (retryable) instead of warning and completing. The replication tail (Pinata/IPFS pinning, gossip, GraphQL push, peer notify) is spawned from the same job but is not awaited: the tail reports(announce, cid_map)back over a oneshot channel, andanchor_ref_updatesruns in the job body only after that report arrives.INSERT ... ON CONFLICT DO NOTHINGagainst a new unique(repo, ref_name, old_sha, new_sha)index creates the durable claim inpendingBEFORE any paid upload; exactly one worker wins, so exactly one worker can ever pay. (2) prepare — the signed item's deterministic ANS-104 id is persisted on the row before the request is sent. (3) upload — outcomes are classified: a definitive provider rejection marks the rowfailed; a connection drop or malformed success leaves ituploading(the item may have been accepted). (4) record — the accepted tx id is persisted as the terminalrecordedstate. A retry that finds a non-terminal claim probes the gateway for the persisted item id first: present → record as-is (no second paid artifact), absent → re-upload, no verdict → fail the job without uploading. The anchor's issuer isstate.node_did, never the pusher.satisfies_thresholdcounts distinct signer DIDs, not signature entries; a repeated signature from one maintainer counts once, so copy-pasted signatures cannot fabricate a threshold.GITLAWB_BUNDLER_TOKENadded;validate()refuses to start with a bundler URL without both a funded account and a payment token (Irys bills at/tx/{token}). An anchoring node must now also set an explicitGITLAWB_ARWEAVE_GATEWAY: the implicitarweave.netdefault is gone, because it silently paired the gateway to the bundler URL and broke/verifyfor production deployments (devnet transactions are not resolvable viaarweave.net). The legacyGITLAWB_IRYS_URLis adopted vialegacy_bundler_url_fallbackonly when the funded account/token pair is also set; a bare legacy URL no longer silently enables anchoring (the node warns and starts with anchoring disabled).devnet.irys.xyz+matic+ devnet gateway) and Production (node2.irys.xyz+ethereum+https://arweave.net) shapes, and the comment documents that anchoring needs the URL, a funded ACCOUNT, the TOKEN, and a gateway on the same network.GITLAWB_BUNDLER_ACCOUNTis the funded account that pays (sent asx-irys-paid-by),GITLAWB_BUNDLER_TOKENis the payment-token slug billed at/tx/{token}(it is NOT an API key and is not sent in the paid-by header), andGITLAWB_ARWEAVE_GATEWAYhas no default.RefCertificategainsseq/prev/pusher_sig/signature_input/content_digest/request_path;arweave_anchorsgainscert_id, renamesirys_tx_id→arweave_tx_id. The released v1 migration stays byte-identical (it has NO cert-chain columns); column work lives in append-only v18/v19, v20 drops the superseded(repo_id, ref_name)unique index (documented one-way, attributed to the v10 migration that created it), v21 adds thepost_receive_jobstable, and v22 carries the anchor outbox:state/item_id/claim_token/claimed_atcolumns,arweave_tx_idmade nullable (a claimed row has no tx yet), dedup of pre-v22 duplicate rows, and the unique per-transition index. The anchors listing reads onlyrecorded(terminal) rows. An upgrade test replays the deployed v1 schema.{payload, s}, base64url Ed25519 over the payload), not JWTs; the middleware verifies the full delegation chain whenX-Ucanis present; read enforcement and per-path visibility rules are wired;GITLAWB_ENFORCE_OWNER_PUSHdefaults off.mask_credential_urldrops userinfo, query, and fragment; used by contracts info, the anchors listing, the gateway-inference log, and the verify error body. Bundler/gateway URLs are built with a structuraljoin_url_paththat preserves the query and rejects fragments; reqwest errors have the URL redacted and bodies truncated.Reviewer checklist coverage
verify_data_item_matches_independent_interop_fixture; flat-tags referencedeep_hash_matches_independent_reference_vector.test_verify_anchor_rejects_tampered_13_field_signature,test_verify_anchor_rejects_tampered_7_field_signature.test_verify_anchor_fails_closed_when_outer_identity_cannot_be_corroborated; DB error masked at the fail-closed lookup.list_anchors_drops_query_and_fragment_credentials,test_verify_anchor_error_does_not_leak_gateway_query_credentials,test_anchor_ref_update_redacts_credentials_in_error_body,test_manifest_anchor_redacts_credentials_in_error_body,test_verify_anchor_interrupted_stream_error_is_masked,redaction_helpers_scrub_urls_and_secrets,server::tests::drops_query_and_fragment_credentials.test_anchor_preserves_bundler_path_prefix,test_anchor_preserves_bundler_query,test_anchor_rejects_fragment_in_bundler_url,test_verify_anchor_preserves_gateway_query,test_verify_anchor_rejects_fragment_in_gateway_url.bundler_url_requires_a_funded_account(config),bundler_url_requires_an_explicit_gateway(config),arweave_gateway_has_no_default_network(config),legacy_irys_url_is_adopted_only_with_funded_account_pair(config),env_example_bundler_block_is_startable,test_anchor_ref_update_rejects_missing_bundler_account(request).post_receive_job_survives_handler_abort(crash-between-enqueue-and-spawn recovery plus idempotent replay),inv22_replication_tail_spawns_at_the_durability_boundary(ordering gate binds enqueue-before-spawn-before-release).anchor_record_failure_is_reconciled_without_double_pay(accepted upload whoserecordedUPDATE fails → row leftuploadingwith its item id; the retry probes the gateway, finds the item, records it WITHOUT a second paid request),anchor_claim_db_failure_never_uploads(claim unanswerable → fail closed, no upload),anchor_probe_failure_never_uploads(gateway probe returns no verdict → fail closed, no upload, row stays non-terminal),post_receive_job_anchor_failure_retries_and_replay_never_reuploads(bundler 500 → rowfailed, the drain probes, re-uploads, records,done; replay never pays twice; the stored anchor'snode_didis the node's, not the pusher's),two_concurrent_workers_claim_the_job_once(atomic job claim → exactly one upload).test_upload_rejects_empty_missing_and_malformed_success_ids(empty/missing/malformed bundler ids areUncertain; a 43-char base64url id isAccepted).satisfies_threshold_rejects_duplicated_signature(a duplicated signature of one key fails a 2-of-3).list_anchors_limit_zero_uses_default_limit(limit=0falls back to the default page size instead of returning nothing),list_anchors_without_gateway_omits_arweave_url(no gateway → no relative/tx_idURL, the durable tx id still lists).upgrade_path_tests::upgrading_released_v1_schema_lands_cert_and_anchor_columns.How a reviewer can verify
DATABASE_URL=postgresql://gitlawb:changeme@localhost:5432/gitlawb cargo test --workspaceAll 1417 tests across the workspace pass (877 in the
gitlawb-nodesuite).cargo fmt --all -- --checkandcargo clippy --workspace --all-targets -- -D warningsare clean.